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

[3.11] gh-93883: elide traceback indicators when possible (GH-93994) #94734

Closed

Conversation

miss-islington
Copy link
Contributor

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]

)

* 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]>
@miss-islington
Copy link
Contributor Author

Status check is done, and it's a success ✅ .

@miss-islington
Copy link
Contributor Author

@belm0 and @pablogsal: Status check is done, and it's a success ❌ .

@pablogsal
Copy link
Member

@belm0 Seems this is going to need a manual backport

@pablogsal
Copy link
Member

Do you mind creating it using cherry_picker and tagging me in the PR?

@DanielNoord
Copy link
Contributor

@belm0 and @pablogsal: Status check is done, and it's a success ❌ .

Opened python/miss-islington#564 to report miss-islington's ambivalence in this comment.

@belm0
Copy link
Contributor

belm0 commented Jul 11, 2022

Do you mind creating it using cherry_picker and tagging me in the PR?

Will do, but I'm not sure what to make of this comment on the original PR-- fixing main branch CI is higher priority?

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

#93994 (comment)

@belm0
Copy link
Contributor

belm0 commented Jul 11, 2022

I think the same tests failed on this backport PR and in master (probably due to new test code that automatic merge missed). I'll fix master first, then backport.

(I've got an appointment now, but will pick this up in about 2 hrs.)

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

Successfully merging this pull request may close these issues.

5 participants