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: 3.11 backtrace visual indicators are too verbose #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:44Z

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 <[email protected]>

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 <[email protected]>

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

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

kumaraditya303 · 2022-07-11T07:26:37Z

This change broke all main branch CI and buildbots. See https://github.com/python/cpython/runs/7277260781?check_suite_focus=true

belm0 · 2022-07-11T08:32:30Z

This change broke all main branch CI and buildbots.

Sorry-- I hadn't rebased in a few weeks, perhaps automatic merge let some new test code through that needs revision. Looking...

PR python#93994 was merged without being rebased in a few weeks, and some new test code using the old scheme passed through automatic merge.

belm0 · 2022-07-11T08:44:01Z

Fix is at #94737 hopefully. I'll be AFK for about 2 hours.

PR #93994 was merged without being rebased in a few weeks, and some new test code using the old scheme passed through automatic merge.

…onGH-93994) * 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 ``` (cherry picked from commit da71751) Co-authored-by: John Belmonte <[email protected]>

…#94737) PR python#93994 was merged without being rebased in a few weeks, and some new test code using the old scheme passed through automatic merge. (cherry picked from commit 1fdc35e)

…onGH-93994) * 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 <[email protected]>

…H-94740) 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 ``` Automerge-Triggered-By: GH:pablogsal

…rmat See python/cpython#93883 and python/cpython#93994 Fixes pytest-dev#10131

…rmat See python/cpython#93883 and python/cpython#93994

belm0 requested a review from iritkatriel as a code owner June 19, 2022 05:04

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

belm0 marked this pull request as draft June 19, 2022 05:05

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

belm0 added 5 commits June 24, 2022 22:30

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 June 24, 2022 13:31

blurb-it bot and others added 4 commits June 24, 2022 14:06

📜🤖 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 June 24, 2022 14:26

belm0 requested a review from terryjreedy as a code owner June 24, 2022 14:26

belm0 mentioned this pull request Jun 24, 2022

3.11 backtrace visual indicators are too verbose #93883

Closed

terryjreedy requested a review from pablogsal June 24, 2022 14:55