classification
Title: IDLE: print "Did you mean?" for AttributeError and NameError
Type: enhancement Stage: resolved
Components: IDLE Versions: Python 3.11, Python 3.10
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: terry.reedy Nosy List: Dennis Sweeney, aroberge, epaine, miss-islington, pablogsal, shreyanavigyan, terry.reedy
Priority: normal Keywords: patch

Created on 2021-05-04 03:49 by Dennis Sweeney, last changed 2021-05-08 01:00 by terry.reedy. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 25912 merged epaine, 2021-05-05 08:50
PR 25973 merged miss-islington, 2021-05-07 23:52
PR 25975 closed terry.reedy, 2021-05-08 00:33
Messages (19)
msg392850 - (view) Author: Dennis Sweeney (Dennis Sweeney) * (Python triager) Date: 2021-05-04 03:49
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
Type "help", "copyright", "credits" or "license()" for more information.
class A:
    foobar = 1

    
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?
msg392854 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2021-05-04 06:06
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
  exec(compile(code, '<pyshell>', 'single', 0x200, True))
Still, 'int.reel' does not give the message while 'int.reel' in the exec above does.  I have no idea right now why the difference.
msg392855 - (view) Author: Shreyan Avigyan (shreyanavigyan) * Date: 2021-05-04 07:34
Python shell uses <stdin> to evaluate while IDLE uses <pyshell>. Is that the problem?
msg392857 - (view) Author: Dennis Sweeney (Dennis Sweeney) * (Python triager) Date: 2021-05-04 08:04
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
pythonrun.c: print_exception(), which is only referenced by
pythonrun.c: print_exception_recursive(), which is only referenced by itself and
pythonrun.c: _PyErr_Display(), which is referenced by:
    - threadmodule.c: thread_excepthook_file()
    - pythonrun.c: PyErr_Display()
pythonrun.c: PyErr_Display() is referenced by:
    - pythonrun.c: _PyErr_PrintEx()
    - pythonrun.c: _Py_FatalErr_PrintExc()
    - _testcapimodule.c: exception_print()
    - sysmodule.c: sys_excepthook_impl() (Python's sys.excepthook)
pythonrun.c: _PyErr_PrintEx() is referenced by:
    - pythonrun.c: PyErr_PrintEx() is referenced by
        - pythonrun.c: PyErr_Print()
        - pylifecycle.c: new_interpreter()
    - pythonrun.c _PyErr_Print() is referenced by
        - ceval.c:_Py_FinishPendingCalls()
        - pylifecycle.c: init_importlib_external()
        - pylifecycle.c: init_interp_main()
msg392859 - (view) Author: Dennis Sweeney (Dennis Sweeney) * (Python triager) Date: 2021-05-04 08:09
PyErr_Display() grabs the current sys.stderr and writes to that, but it looks like IDLE never gets to call PyErr_Display().
msg392861 - (view) Author: Dennis Sweeney (Dennis Sweeney) * (Python triager) Date: 2021-05-04 08:23
It looks like Lib/idlelib/run.py : print_exception() re-implements the traceback, rather than relying on sys.excepthook
msg392863 - (view) Author: Dennis Sweeney (Dennis Sweeney) * (Python triager) Date: 2021-05-04 08:43
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:
msg392864 - (view) Author: Shreyan Avigyan (shreyanavigyan) * Date: 2021-05-04 08:44
Does the test suite run succesfully?
msg392872 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2021-05-04 10:52
(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.
msg392878 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-05-04 11:38
> Pablo, I checked an AttributeError instance and the missing phrase is not present

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.
msg392879 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-05-04 11:39
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.
msg392933 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2021-05-04 18:09
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

1. add sys.tracebackhook, to be called by the traceback display functions if not None.  Then idlelib.run.print_exception would not have to duplicate the sometimes changing logic for chaining or otherwise joining multiple tracebacks into one mega-traceback.  Instead, from what Dennis said above, it could set the hook and call the built-in exceptionhook.

2. add a new optional parameter to exceptionhook and the traceback display function.  This would have the same advantage as above.
msg392939 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-05-04 19:55
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 :)
msg392945 - (view) Author: Dennis Sweeney (Dennis Sweeney) * (Python triager) Date: 2021-05-04 20:16
Another idea: do what test_exceptions() does:

        try:
            f()
        except NameError as exc:
            with support.captured_stderr() as err:
                sys.__excepthook__(*sys.exc_info())

        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.
msg392966 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2021-05-04 23:40
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?
msg392968 - (view) Author: Dennis Sweeney (Dennis Sweeney) * (Python triager) Date: 2021-05-05 01:28
I unfortunately don't have the time/internet access this week to do a PR.
msg393055 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2021-05-05 23:51
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.
msg393232 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2021-05-08 00:35
New changeset 5a5237c6d08ed97458b0903d6836624168df0b51 by Miss Islington (bot) in branch '3.10':
bpo-44026: Idle - display interpreter's 'did you mean' hints (GH-25912)
https://github.com/python/cpython/commit/5a5237c6d08ed97458b0903d6836624168df0b51
msg393233 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2021-05-08 01:00
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.
History
Date User Action Args
2021-05-08 01:00:19terry.reedysetstatus: open -> closed
resolution: fixed
stage: resolved
2021-05-08 01:00:05terry.reedysetmessages: + msg393233
stage: patch review -> (no value)
2021-05-08 00:35:32terry.reedysetmessages: + msg393232
2021-05-08 00:33:41terry.reedysetpull_requests: + pull_request24631
2021-05-07 23:52:11miss-islingtonsetnosy: + miss-islington

pull_requests: + pull_request24629
stage: patch review
2021-05-05 23:51:55terry.reedysetmessages: + msg393055
stage: patch review -> (no value)
2021-05-05 14:52:49arobergesetnosy: + aroberge
2021-05-05 08:50:19epainesetkeywords: + patch
nosy: + epaine

pull_requests: + pull_request24581
stage: patch review
2021-05-05 01:28:28Dennis Sweeneysetmessages: + msg392968
2021-05-04 23:40:47terry.reedysetmessages: + msg392966
title: IDLE doesn't offer "Did you mean?" for AttributeError and NameError -> IDLE: print "Did you mean?" for AttributeError and NameError
2021-05-04 20:16:22Dennis Sweeneysetmessages: + msg392945
2021-05-04 19:55:17pablogsalsetmessages: + msg392939
2021-05-04 18:09:11terry.reedysetmessages: + msg392933
2021-05-04 11:39:17pablogsalsetmessages: + msg392879
2021-05-04 11:38:03pablogsalsetmessages: + msg392878
2021-05-04 10:52:19terry.reedysetmessages: + msg392872
2021-05-04 08:44:23shreyanavigyansetmessages: + msg392864
2021-05-04 08:43:17Dennis Sweeneysetmessages: + msg392863
2021-05-04 08:23:41Dennis Sweeneysetmessages: + msg392861
2021-05-04 08:09:42Dennis Sweeneysetmessages: + msg392859
2021-05-04 08:04:05Dennis Sweeneysetmessages: + msg392857
2021-05-04 07:34:10shreyanavigyansetnosy: + shreyanavigyan
messages: + msg392855
2021-05-04 06:06:37terry.reedysetmessages: + msg392854
2021-05-04 03:49:43Dennis Sweeneysetnosy: + pablogsal
2021-05-04 03:49:13Dennis Sweeneycreate