Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TST: DocTestFailure 'MaskedArray' object has no attribute 'unit' #14005

Closed
pllim opened this issue Nov 14, 2022 · 13 comments
Closed

TST: DocTestFailure 'MaskedArray' object has no attribute 'unit' #14005

pllim opened this issue Nov 14, 2022 · 13 comments

Comments

@pllim
Copy link
Member

pllim commented Nov 14, 2022

Hold your horses, @astrofrog and @WilliamJamieson ! We need to fix this before merging more black stuff. I think one of them broke a doctest. The PRs might be fine individually but maybe didn't play well together. cc @mhvk

Example log: https://github.com/astropy/astropy/actions/runs/3465731975/jobs/5788828409

_____________________________ [doctest] index.rst ______________________________
119 |Quantity|, but does not share any of its methods.  Hence, even though the
120 resulting class looks reasonable at first glance, it does not work as expected::
121 
122   >>> q = [1., 2.] * u.m
123   >>> np_mq = np.ma.MaskedArray(q, mask=[False, True])
124   >>> np_mq
125   masked_Quantity(data=[1.0, --],
126                   mask=[False,  True],
127             fill_value=1e+20)
128   >>> np_mq.unit
Differences (unified diff with -expected +actual):
    @@ -1,3 +1,6 @@
     Traceback (most recent call last):
    -...
    -AttributeError: 'MaskedArray' object has no attribute 'unit'
    +  File "/opt/hostedtoolcache/Python/3.10.8/x64/lib/python3.10/doctest.py", line 1350, in __run
    +    exec(compile(example.source, filename, "single",
    +  File "<doctest index.rst[21]>", line 1, in <module>
    +    np_mq.unit
    +AttributeError: 'MaskedArray' object has no attribute 'unit'. Did you mean: '_unit'?

/home/runner/work/astropy/astropy/docs/utils/masked/index.rst:128: DocTestFailure
@pllim pllim added this to the v5.2 milestone Nov 14, 2022
@WilliamJamieson
Copy link
Contributor

So I can reproduce this locally now using python 3.10 in a brand new environment.

I have tested every (merge) commit back to 0c37a71, the merge commit for #13484 and found this error persists that far back. Since #13484 occurred before any of the black commits. It is safe to assume that this issue is independent of any of the black changes and so must be something else.

@WilliamJamieson
Copy link
Contributor

I carefully reviewed the two CI runs related to this failure:

Since #14005 (comment) indicated that the error is not related to the merge, I went through the pip-freeze in both cases.

The only difference I could see is the failing CI used exceptiongroup==1.0.3 while the passing CI used exceptiongroup==1.0.2 I have confirmed that manually downgrading to exceptiongroup==1.0.2 locally fixes the failing test, and then upgrading to exceptiongroup==1.0.3 reproduces the failure.

Thus I believe that exceptiongroup==1.0.3 (release today Nov 14, 2022) is responsible for this CI failure.

@WilliamJamieson
Copy link
Contributor

It looks like the changes causing this issue originate in agronholm/exceptiongroup#42.

Should we just pin exeptiongroup<1.0.3 for now?

@jdavies-st
Copy link
Contributor

jdavies-st commented Nov 15, 2022

It looks as though the exception type is the same, but the message has changed. Given that exception type and message are the only things that doctest cares about, one could also ignore the exception detail, especially if this is Python 3.12 behavior being backported.

https://docs.python.org/3/library/doctest.html#what-about-exceptions
https://docs.python.org/3/library/doctest.html#doctest.IGNORE_EXCEPTION_DETAIL

I.e.

>>> np_mq.unit  # doctest: +IGNORE_EXCEPTION_DETAIL

Especially if this is the only test affected. From reading the changes in exceptiongroup==1.0.3, it seems this is Python 3.12 behavior that 3.11 doesn't have yet.

It might be worth filing an issue indicating that this exceptiongroup==1.0.3 change breaks tests that care about consistent exception messages between < 3.11 and 3.11. Not good.

@jdavies-st
Copy link
Contributor

Actually something more insidious and confusing is happening.

If I do the following in Python 3.11:

>>> class Foo:
...     def __init__(self):
...         pass
...     
...     def _bar(self):
...         return 42
... 
>>> foo = Foo()
>>> foo.bar
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'Foo' object has no attribute 'bar'. Did you mean: '_bar'?

I get the suggestion. Fun! So this is already baked into Python 3.11. But when I then make this into a module

# foo.py

class Foo:
    def __init__(self):
        pass
    
    def _bar(self):
        return 42
    

def func():
    """
    This docstring tests Foo

    Example
    -------

    >>> foo = Foo()
    >>> foo.bar
    Traceback (most recent call last):
    ...
    AttributeError: 'Foo' object has no attribute 'bar'. Did you mean: '_bar'?
    """
    pass

and run pytest and w/doctest on it (under Python 3.11), it fails:

$ pytest foo.py --doctest-modules
============================================= test session starts ==============================================
platform darwin -- Python 3.11.0, pytest-7.2.0, pluggy-1.0.0
rootdir: /Users/jdavies/dev
collected 1 item                                                                                               

foo.py F                                                                                                 [100%]

=================================================== FAILURES ===================================================
______________________________________________ [doctest] foo.func ______________________________________________
013 
014     This docstring tests Foo
015 
016     Example
017     -------
018 
019     >>> foo = Foo()
020     >>> foo.bar
Differences (unified diff with -expected +actual):
    @@ -1,3 +1,6 @@
     Traceback (most recent call last):
    -...
    -AttributeError: 'Foo' object has no attribute 'bar'. Did you mean: '_bar'?
    +  File "/Users/jdavies/miniconda3/envs/astropy/lib/python3.11/doctest.py", line 1350, in __run
    +    exec(compile(example.source, filename, "single",
    +  File "<doctest foo.func[1]>", line 1, in <module>
    +    foo.bar
    +AttributeError: 'Foo' object has no attribute 'bar'

/Users/jdavies/dev/foo.py:20: DocTestFailure
=========================================== short test summary info ============================================
FAILED foo.py::foo.func
============================================== 1 failed in 0.02s ===============================================

So pytest w/doctest actually gives me the wrong exception message under Python 3.11. This is a bug.

Meanwhile, if I do the same exercise above under Python 3.10:

>>> class Foo:
...     def __init__(self):
...         pass
...     
...     def _bar(self):
...         return 42
... 
>>> foo = Foo()
>>> foo.bar
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'Foo' object has no attribute 'bar'. Did you mean: '_bar'?

I also get the suggestion. But when I run pytest w/doctest, again under Python 3.10:

$ pytest foo.py --doctest-modules
============================================= test session starts ==============================================
platform darwin -- Python 3.10.8, pytest-7.2.0, pluggy-1.0.0
rootdir: /Users/jdavies/dev
collected 1 item                                                                                               

foo.py .                                                                                                 [100%]

============================================== 1 passed in 0.02s ===============================================

everything passes.

So TL;DR, it seems that Python 3.11 with pytest is the thing that is broken right now. It must strip away suggestions or something.

@jdavies-st
Copy link
Contributor

jdavies-st commented Nov 15, 2022

Btw, these NameError and AttributeError suggestions were first implemented in Python 3.10, though the suggestion text is not part of the actual traceback, it is computed on-the-fly and added. Which explains why pytest w/doctest doesn't actually pick it up.

For 3.12 it is apparently supposed to be in the traceback itself, not computed on-the-fly, but this is not yet the case in 3.11.0. The example below shows this:

>>> class Foo:
...     def __init__(self):
...         pass
...     
...     def _bar(self):
...         return 42
... 
>>> foo = Foo()
>>> foo.bar
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'Foo' object has no attribute 'bar'. Did you mean: '_bar'?
>>> try:
...     foo.bar
... except AttributeError as e:
...     print(e)
... 
'Foo' object has no attribute 'bar'

@cfbolz
Copy link

cfbolz commented Nov 15, 2022

I don't know yet why stuff breaks with pytest, if exceptiongroup is to blame I'm happy to look into that more. Just chiming in to say that it's very much intentional on the CPython side that the suggestions are only computed when printing the traceback. computing the suggestion is relatively costly (it does dir(obj), then computes an edit distance between the wrong attribute and all other attributes) so it would be way too expensive to do that computation on every instantiation of AttributeError, some of which might be caught and never even reach the user.

@cfbolz
Copy link

cfbolz commented Nov 15, 2022

right, I think I see. so the problem is that CPython has two different exception printing mechanisms, one is the traceback module, the other one is built into the interpreter, written in C. doctest uses the former, the REPL the latter. in 3.10 and 3.11 the two got out of sync, because Name/AttributeError suggestions only got added to the C code. This fixed in this PR but was deemed "not a bug" but a new feature for traceback and thus only added to 3.12. So doctest for suggestions is indeed broken:

$ ./python -m doctest foo.py # 3.12 alpha passes
$ python3.11 -m doctest foo.py 
**********************************************************************
File "/home/cfbolz/projects/cpython/foo.py", line 17, in foo.func
Failed example:
    foo.bar
Expected:
    Traceback (most recent call last):
    ...
    AttributeError: 'Foo' object has no attribute 'bar'. Did you mean: '_bar'?
Got:
    Traceback (most recent call last):
      File "/usr/lib/python3.11/doctest.py", line 1350, in __run
        exec(compile(example.source, filename, "single",
      File "<doctest foo.func[1]>", line 1, in <module>
        foo.bar
    AttributeError: 'Foo' object has no attribute 'bar'
**********************************************************************
1 items had failures:
   1 of   2 in foo.func
***Test Failed*** 1 failures.
$ python3.10 -m doctest foo.py 
**********************************************************************
File "/home/cfbolz/projects/cpython/foo.py", line 17, in foo.func
Failed example:
    foo.bar
Expected:
    Traceback (most recent call last):
    ...
    AttributeError: 'Foo' object has no attribute 'bar'. Did you mean: '_bar'?
Got:
    Traceback (most recent call last):
      File "/usr/lib/python3.10/doctest.py", line 1350, in __run
        exec(compile(example.source, filename, "single",
      File "<doctest foo.func[1]>", line 1, in <module>
        foo.bar
    AttributeError: 'Foo' object has no attribute 'bar'
**********************************************************************
1 items had failures:
   1 of   2 in foo.func
***Test Failed*** 1 failures.

so now exceptiongroup gets involved 😅. It monkeypatches traceback on 3.10 to support exception groups. It does this to be able to use traceback for sys.excepthook without breaking suggestions. As of yesterday (1.0.3) it fixes the bug in CPython's traceback as a side-effect, but only on 3.10. On 3.11 it doesn't do any monkeypatching, so the arguable bug in 3.11 of not printing suggestions when using traceback (like doctests do) is still there.

Sorry for the wall of text. All this to say that a pragmatic solution would be to ignore the tail end of the error message in the doctest and wait for 3.12

@jdavies-st
Copy link
Contributor

Exactly. Thanks for distilling that. 🙌

And because there are lots of docstring examples in the wild that are tested daily via CI by pytest and doctest that have never had to be updated for NameError/AttributeError suggestions, I suspect that the traceback monkeypatching in 1.0.3 will probably break them for <=3.10 but not 3.11. I suppose this could break regular tests (outside of doctests) as well, if the error message is checked in full.

Agree the short-term pragmatic answer is to use +IGNORE_EXCEPTION_DETAIL in the docstring example, which actually will ignore the whole message.

@cfbolz
Copy link

cfbolz commented Nov 15, 2022

or you could do this kind of thing:

    >>> foo.bar # doctest: +ELLIPSIS
    Traceback (most recent call last):
    ...
    AttributeError: 'Foo' object has no attribute 'bar'...

note the ... at the end. this still checks the important part of the message but ignores the suggestions.

@WilliamJamieson
Copy link
Contributor

@jdavies-st and @cfbolz thanks for figuring out the root cause of the issue. The final solution of just adding an ... is nice, and it makes sense given the new (and useful) traceback/exception messages.

@mhvk
Copy link
Contributor

mhvk commented Nov 15, 2022

Thanks all for finding and fixing this issue!

I think it can also be closed -- if we want something more general probably good to open another issue.

@mhvk mhvk closed this as completed Nov 15, 2022
@pllim
Copy link
Member Author

pllim commented Nov 15, 2022

Accepted solution LGTM. Belated thanks to everyone involved to help resolve this problem so quickly!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants