-
-
Notifications
You must be signed in to change notification settings - Fork 31.1k
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
IDLE: print "Did you mean?" for AttributeError and NameError #88192
Comments
After bpo-38530, I get this in the python shell: Python 3.10.0b1 (tags/v3.10.0b1:ba42175, May 3 2021, 20:22:30) [MSC v.1928 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> class A:
... foobar = 1
...
>>> A.foocar
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
AttributeError: type object 'A' has no attribute 'foocar'. Did you mean: 'foobar'?
>>> But I get this in IDLE: Python 3.10.0b1 (tags/v3.10.0b1:ba42175, May 3 2021, 20:22:30) [MSC v.1928 64 bit (AMD64)] on win32
A.foocar
Traceback (most recent call last):
File "<pyshell#3>", line 1, in <module>
A.foocar
AttributeError: type object 'A' has no attribute 'foocar' Can we extend this functionality to IDLE, and fix the discrepancy? |
I am surprised for 2 reasons. First, I have seen other improved messages in IDLE, though maybe only for syntax errors. Second, code entered through IDLE is executed by Python, 'same' in this respect as code entered through the REPL. In more detail, IDLE calls |
Python shell uses <stdin> to evaluate while IDLE uses <pyshell>. Is that the problem? |
I'm not sure if this helps, but this is the relevant tree of callers: suggestions.c: _Py_Offer_Suggestions() (the expected behavior) is only referenced by |
PyErr_Display() grabs the current sys.stderr and writes to that, but it looks like IDLE never gets to call PyErr_Display(). |
It looks like Lib/idlelib/run.py : print_exception() re-implements the traceback, rather than relying on sys.excepthook |
Indeed, this change enables the feature for IDLE, though I'm not sure what it breaks. diff --git a/Lib/idlelib/run.py b/Lib/idlelib/run.py
index 07e9a2bf9c..319b16f311 100644
--- a/Lib/idlelib/run.py
+++ b/Lib/idlelib/run.py
@@ -569,7 +569,7 @@ def runcode(self, code):
self.user_exc_info = sys.exc_info() # For testing, hook, viewer.
if quitting:
exit()
- if sys.excepthook is sys.__excepthook__:
+ if False:
print_exception()
else:
try: |
Does the test suite run succesfully? |
(Shreyan, the fake filenames are not relevant.) By default, sys.excepthook is sys.__excepthook is <built-in function excepthook>. So by default IDLE runcode calls print_exception, which call cleanup_traceback for each traceback printed. This function makes two important changes to the traceback. First, it removes traceback lines that are not present when running in the C-coded REPL, because IDLE does some functions with Python code. The result is that IDLE tracebacks nearly always equal REPL tracebacks. Second, for Shell code, IDLE add cached code lines. Compare >>> a = b
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
NameError: name 'b' is not defined to >>> a = b
Traceback (most recent call last):
File "<pyshell#9>", line 1, in <module>
a = b
NameError: name 'b' is not defined
>>> We are not going to stop using print_exception. Note that it is normal for IDEs to modify tracebacks. The 'is' is false when user change excepthook. The false branch was recently added to run user excepthooks. Pablo, I checked an AttributeError instance and the missing phrase is not present. Why is it hidden from Python code. Why not add the rest of the message to the message? Or at least add it to the tuple or an attribute. Or add a .get_suggestion method to exception objects, like .with_traceback. |
This cannot be included in the AttributeError because of performance reasons. These errors will be thrown naturally all over the place without this meaning that the interpreter will finish so we can only compute these safely on the except hook. We also discarded mutating the attribute error because it can make some stuff crash if the exception happens on a thread and that doesn't imply full program termination. |
I don't feel comfortable adding a public API to get the message either in the traceback or the exception, at least for the time being. |
I realized after posting that looking for close matching is a performance issue and does not need to be done unless the exception is going to be displayed. But it is a shame to keep such great work away from many users. What would you think of either
|
Those are intesting options, I will think about this. I am still afraid of adding more APIs in this area as having arbitrary Python getting call in the middle of handling the exceptions it can be quite tricky: think that we are handling an exception already so other exceptions can mess up stuff in very subtle ways or we need to ignore exceptions and that can also be dangerous. Another thing we could do is reproduce the feature in idle itself with a custom display hook, which would be easier as we can just use difflib. It would be slightly different than the built-in one but honestly I don't think that's a problem at all. I understand that other people may think differently so I am open minded as well. But I wanted to clarify what are my worries :) |
Another idea: do what test_exceptions() does:
self.assertNotIn("a1", err.getvalue()) Then instead of the assert, do something like last_line = err.getvalue().rsplit("\n", 2)[-2]
_, did_you_mean, suggestion = last_line.rpartition("Did you mean: ")
if did_you_mean:
print(did_you_mean + suggestion) This can probably be done without test.support. |
Pablo, unless you are suggesting rewriting IDLE's custom exception handing in C, I don't see how making it a single function would be an improvement. As long as it is Python code, the problem is accessing the message supplement. After reading your comment, I agree that injecting Python code into the C exception handling is a bad idea. It also, I see now, not needed run.print_exception first calls traceback.extract_tb (line 238). The doc says " It is useful for alternate formatting of stack traces", which IDLE does. print_exception next calls the undocumented traceback.print_list, which is format_list + print*. Finally, print_exception calls traceback.format_exception_only (line 2440), which returns a list of lines that is immediately printed. So all IDLE needs is for format_exception_only to include the extra lines, either by default or by option. But it just calls TracebackException.format_exception)only, which call a private Python-coded function, which can only add the lines if accessible from Python-code. Dennis cleverly pointed out that such access is available, even if awkwardly, by capturing the standard excepthook outout and keeping only the missing part. (test.support.captured_stderr just wraps io.StringIO as a context manager. IDLE can borrow and edits its code.) Dennis, would you like to prepare a PR that follows the format_exception_only call with something like if isinstance(typ, (AttributeError, NameError)):
lines.extend(extract_extra_message(<args>)) and a definition of the new function? (and a new test)? Or perhaps better, replace format_exception_only with a function that gets all lines at once. Pablo, do any other exception types have such omitted help info? Do you have plans for any more? |
I unfortunately don't have the time/internet access this week to do a PR. |
This is only backported to 3.10 and not 3.9 because the fix recommendations were only added in 3.10. If the code difference became a problem, this could be backported because the roundabout method for name and attribute errors would work in 3.9. |
Merged to 3.11.0a0 first, bot forget to post it. Dennis, thank you for the analysis and then the suggestion as to how to access the not directly accessible. It would likely have been awhile before I stumbled from 'cannot' to 'can with workaround'. Feel free to add ideas on other IDLE issues. Thanks EP for making the fix work even with chained exceptions *and* for providing tests. I redid half the lines, but core test logic was correct and remains. In .0b1+ repository (and future .0b2 release) IDLE: >>> try: abc
... except NameError: f"{complex.reel(1+1j)} errors occurred!"
...
Traceback (most recent call last):
File "<pyshell#2>", line 1, in <module>
try: abc
NameError: name 'abc' is not defined. Did you mean: 'abs'?
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "<pyshell#2>", line 2, in <module>
except NameError: f"{complex.reel(1+1j)} errors occurred!"
AttributeError: type object 'complex' has no attribute 'reel'. Did you mean: 'real'? And thank you Pablo for making exception messages more helpful. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: