gh-93883: elide traceback indicators when possible #93994

belm0 · 2022-06-19T05:04:53Z

Elide traceback column indicators when the entire line of the
frame is implicated. This reduces traceback length and draws
more attention to the remaining (very relevant) indicators.

Example:

Traceback (most recent call last):
  File "query.py", line 99, in <module>
    bar()
  File "query.py", line 66, in bar
    foo()
  File "query.py", line 37, in foo
    magic_arithmetic('foo')
  File "query.py", line 18, in magic_arithmetic
    return add_counts(x) / 25
           ^^^^^^^^^^^^^
  File "query.py", line 24, in add_counts
    return 25 + query_user(user1) + query_user(user2)
                ^^^^^^^^^^^^^^^^^
  File "query.py", line 32, in query_user
    return 1 + query_count(db, response['a']['b']['c']['user'], retry=True)
                               ~~~~~~~~~~~~~~~~~~^^^^^
TypeError: 'NoneType' object is not subscriptable

TODO:

C traceback implementation
update documentation

Issue: gh-93883

See also the original feature issue: gh-88116.

arhadthedev · 2022-06-19T07:46:19Z

For backlinking purposes the parent issue is gh-93883 (until the link autoiinsertion is returned back; cannot find that PR).

Elide traceback column indicators when the entire line of the frame is implicated. This reduces traceback length and draws even more attention to the remaining (very relevant) indicators. Example: ``` Traceback (most recent call last): File "query.py", line 99, in <module> bar() File "query.py", line 66, in bar foo() File "query.py", line 37, in foo magic_arithmetic('foo') File "query.py", line 18, in magic_arithmetic return add_counts(x) / 25 ^^^^^^^^^^^^^ File "query.py", line 24, in add_counts return 25 + query_user(user1) + query_user(user2) ^^^^^^^^^^^^^^^^^ File "query.py", line 32, in query_user return 1 + query_count(db, response['a']['b']['c']['user'], retry=True) ~~~~~~~~~~~~~~~~~~^^^^^ TypeError: 'NoneType' object is not subscriptable ``` WORK IN PROGRESS TODO: * C traceback implementation * update documentation

Rather than going out of our way to provide indicator coverage in every traceback test suite, the indicator test suite should be responible for sufficient coverage (e.g. by adding a basic exception group test to ensure that margin strings are covered).

terryjreedy · 2022-06-24T15:02:43Z

I approve of the idea of the change and, of course, adjusting the IDLE test.

I would like to see this backported as I consider the noise a visual bug in the new feature.

Lib/test/test_doctest.py

pablogsal · 2022-06-27T17:43:34Z

Lib/test/test_traceback.py

@@ -387,17 +387,16 @@ def get_exception(self, callable):

    def test_basic_caret(self):
        def f():
-            raise ValueError("basic caret tests")
+            if True: raise ValueError("basic caret tests")


Haha, this is a bit confusing the first time I just saw it. I understand that this is so the raise doesn't get elided because is fully covered, but maybe we can do the same with a ternary operation raise ValueError("blech") or ValueError("never executed"). I don't have a string opinion to be honest, just that is a bit confusing :)

Rightly noted. I had a pending commit to explain that idiom. Let's see how it looks after adding that.

Lib/test/test_traceback.py

pablogsal · 2022-06-27T17:46:45Z

Lib/test/test_traceback.py

@@ -1459,10 +1457,8 @@ def exc():
             f'  + Exception Group Traceback (most recent call last):\n'
             f'  |   File "{__file__}", line {self.callable_line}, in get_exception\n'
             f'  |     exception_or_callable()\n'
-             f'  |     ^^^^^^^^^^^^^^^^^^^^^^^\n'


I am a bit concerned that we are missing some test coverage with exception groups + debug ranges here. We should make sure we have at least one specific exception group test that covers debug ranges as well.

Yes-- that was my intention with the added test_caret_exception_group() test. I thought it would be better to leave the onus on the CaretTests suite.

Python/traceback.c

Co-authored-by: Pablo Galindo Salgado <Pablogsal@gmail.com>

* explain "if True:" idiom for caret tests * document test_caret_exception_group() test * add eliding explanation to summary in traceback.c

belm0 · 2022-06-28T00:30:29Z

@pablogsal would you add this PR to the summary table at #88116 (comment) ?

belm0 · 2022-07-10T22:23:38Z

@pablogsal hoping it will only take a few minutes of your time to confirm the requested changes, and possible to packport this to 3.11 in time for the final release 🙏. Thank you!

pablogsal

Changes LGTM. I'm merging this PR so it makes it in today's release.

Great work @belm0 🎉🚀 And thanks for the patience ;)

miss-islington · 2022-07-11T06:42:55Z

Thanks @belm0 for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

) * pythongh-93883: elide traceback indicators when possible Elide traceback column indicators when the entire line of the frame is implicated. This reduces traceback length and draws even more attention to the remaining (very relevant) indicators. Example: ``` Traceback (most recent call last): File "query.py", line 99, in <module> bar() File "query.py", line 66, in bar foo() File "query.py", line 37, in foo magic_arithmetic('foo') File "query.py", line 18, in magic_arithmetic return add_counts(x) / 25 ^^^^^^^^^^^^^ File "query.py", line 24, in add_counts return 25 + query_user(user1) + query_user(user2) ^^^^^^^^^^^^^^^^^ File "query.py", line 32, in query_user return 1 + query_count(db, response['a']['b']['c']['user'], retry=True) ~~~~~~~~~~~~~~~~~~^^^^^ TypeError: 'NoneType' object is not subscriptable ``` Rather than going out of our way to provide indicator coverage in every traceback test suite, the indicator test suite should be responible for sufficient coverage (e.g. by adding a basic exception group test to ensure that margin strings are covered). (cherry picked from commit da71751) Co-authored-by: John Belmonte <john@neggie.net>

bedevere-bot · 2022-07-11T06:43:11Z

GH-94734 is a backport of this pull request to the 3.11 branch.

belm0 requested a review from iritkatriel as a code owner Jun 19, 2022

bedevere-bot added the awaiting review label Jun 19, 2022

belm0 marked this pull request as draft Jun 19, 2022

belm0 force-pushed the trackback_elide_indicators branch 2 times, most recently from 9e77485 to f8b316f Compare Jun 19, 2022

belm0 added 5 commits Jun 24, 2022

fix test_idle

bba12b1

traceback.c: fix doc typo; simplify expression

6a8f56d

traceback.c implementation

94be186

fix test_cmd_line_script

c06257b

belm0 force-pushed the trackback_elide_indicators branch from 472339e to c06257b Compare Jun 24, 2022

blurb-it bot and others added 4 commits Jun 24, 2022

📜🤖 Added by blurb_it.

87fafa6

remove stray maxDiff from test

a64635c

change testing strategy

361da86

Rather than going out of our way to provide indicator coverage in every traceback test suite, the indicator test suite should be responible for sufficient coverage (e.g. by adding a basic exception group test to ensure that margin strings are covered).

update doc examples

d392332

belm0 marked this pull request as ready for review Jun 24, 2022

belm0 requested a review from terryjreedy as a code owner Jun 24, 2022

belm0 mentioned this pull request Jun 24, 2022

3.11 backtrace visual indicators are too verbose #93883

Open

terryjreedy requested a review from pablogsal Jun 24, 2022