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

issue #41: backport the pure python suggestion code from 3.12 #42

Merged
merged 6 commits into from Nov 14, 2022

Conversation

cfbolz
Copy link
Contributor

@cfbolz cfbolz commented Nov 14, 2022

First attempt. Code is from here: python/cpython@bbc7cd6

I suppose a few more tests would be good?

otherwise NameError/AttributeError suggestions break when using
exceptiongroup on 3.10 (and on PyPy)
@agronholm
Copy link
Owner

I suppose a few more tests would be good?

Passing tests would be good :)

@cfbolz
Copy link
Contributor Author

cfbolz commented Nov 14, 2022

That too ;-)

@cfbolz
Copy link
Contributor Author

cfbolz commented Nov 14, 2022

of course it passes locally on 3.10 and pypy, otherwise I wouldn't have pushed. how do you usually run tests locally?

@agronholm
Copy link
Owner

of course it passes locally on 3.10 and pypy, otherwise I wouldn't have pushed. how do you usually run tests locally?

I usually just run tox -p before pushing.

@cfbolz
Copy link
Contributor Author

cfbolz commented Nov 14, 2022

so 3.11 fails, because there exceptiongroup.format_exception etc just defer to the traceback module, which does not do suggestions in 3.11 yet. I could 1) skip the test there or 2) change the logic in __init__.py to use 3.12 as the cutoff, because traceback.py will have suggestion support there.

@agronholm
Copy link
Owner

I'm a bit confused; the test suite should not be calling exceptiongroup.format_exception() at all on Python 3.11. Instead it should be calling traceback.format_exception(). Since nothing is being patched on 3.11, it should "just work", yes?

@agronholm
Copy link
Owner

Ok so, let me change the question. With exceptiongroup not patching anything on 3.11, it also won't break anything, right? If that is so, then skipping the test on 3.11 seems like the right thing to do.

no monkeypatching is done there anyway
@coveralls
Copy link

@cfbolz
Copy link
Contributor Author

cfbolz commented Nov 14, 2022

With exceptiongroup not patching anything on 3.11, it also won't break anything, right? If that is so, then skipping the test on 3.11 seems like the right thing to do.

Yes, that reasoning makes a lot of sense, I just did that.

@agronholm
Copy link
Owner

Goodie! Anything missing from the PR? I see it's still marked as draft.

@cfbolz cfbolz marked this pull request as ready for review November 14, 2022 20:25
@cfbolz
Copy link
Contributor Author

cfbolz commented Nov 14, 2022

no, good from my side

@agronholm agronholm merged commit ebb4582 into agronholm:main Nov 14, 2022
@agronholm
Copy link
Owner

Thanks!

@cfbolz
Copy link
Contributor Author

cfbolz commented Nov 14, 2022

awesome, thank you!

@jdavies-st
Copy link

So currently in Python 3.11.0, the suggestions for NameError and AttributeError still seem to be computed on-the-fly and are not a part of the traceback. An example:

>>> class Foo:
...     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'

Is this supposed to change during some patch release of 3.11.x?

@agronholm
Copy link
Owner

From what I understood, this would be changing in Python 3.12. It's a non-issue on 3.11 as this project doesn't do monkey patching on that version (or any subsequent versions).

@jdavies-st
Copy link

Thanks. Then perhaps the issue we are seeing in astropy/astropy#14005 is actually an upstream usage of exceptiongroup by pytest. In terms of NameError and AttributeError suggestions, Python 3.10 and 3.11 should be treated the same, but now 3.10 is being patched (to work like 3.12), but 3.11 is not.

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

Successfully merging this pull request may close these issues.

None yet

4 participants