FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-42482: remove reference to exc_traceback from TracebackException by iritkatriel · Pull Request #23531 · python/cpython · GitHub

Repository navigation

bpo-42482: remove reference to exc_traceback from TracebackException - #23531

Merged
miss-islington merged 7 commits into
python:masterfrom
iritkatriel:bpo-42482
Dec 1, 2020
Merged

miss-islington merged 7 commits into
python:masterfrom
iritkatriel:bpo-42482

Conversation

iritkatriel commented Nov 27, 2020 •
edited by miss-islington
Loading

Copy link
Copy Markdown
Member

https://bugs.python.org/issue42482

Automerge-Triggered-By: GH:gvanrossum

gvanrossum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Yeah!

Copy link
Copy Markdown
Member Author

Yeah!

Wait, I made a mistake in the refcount test. The TracebackException object is an intermediate, I need to assign it to something.

gvanrossum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Sorry I didn't catch that. :-(

Let me know when you are confident enough that I can merge this.

Copy link
Copy Markdown
Member Author

I think it’s ready now.

Copy link
Copy Markdown
Member Author

BTW - the faulty test did fail on the old code. It must be that the exc_traceback field was closing a cycle with the test function’s frame or something like that.

Copy link
Copy Markdown
Member Author

Actually it might make more sense to move the line that prints the "traceback header" to StackSummary.format(), where the traceback itself is being formatted.

Copy link
Copy Markdown
Member

Actually it might make more sense to move the line that prints the "traceback header" to StackSummary.format(), where the traceback itself is being formatted.

Except that would change the behavior of TracebackException.format(), which feels like a public(ish) API.

iritkatriel commented Nov 29, 2020 •
edited
Loading

Copy link
Copy Markdown
Member Author

Actually it might make more sense to move the line that prints the "traceback header" to StackSummary.format(), where the traceback itself is being formatted.

Except that would change the behavior of TracebackException.format(), which feels like a public(ish) API.

Yes, I'm adding a with_header=False arg to format(). Alternatively, we could leave the header line where it is but under "if self.stack:" and remove the _print_traceback_header boolean.

iritkatriel commented Nov 29, 2020 •
edited
Loading

Copy link
Copy Markdown
Member Author

BTW - the faulty test did fail on the old code. It must be that the exc_traceback field was closing a cycle with the test function’s frame or something like that.

This is the cause - the TracebackException was not really intermediate because format() returns a generator:

(Pdb) gc.get_referrers(gc.get_referrers(exc_info[2])[2])
[<traceback.TracebackException object at 0x000001E0D48E3A90>]
(Pdb) gc.get_referrers(gc.get_referrers(gc.get_referrers(exc_info[2])[2])[0])
[<frame at 0x000001E0D2551C90, file 'C:\Users\User\src\cpython\lib\traceback.py', line 616, code format>]
(Pdb) gc.get_referrers(gc.get_referrers(gc.get_referrers(gc.get_referrers(exc_info[2])[2])[0])[0])
[<generator object TracebackException.format at 0x000001E0D49E63C0>]
(Pdb) gc.get_referrers(gc.get_referrers(gc.get_referrers(gc.get_referrers(gc.get_referrers(exc_info[2])[2])[0])[0])[0])
[{'self': <test.test_traceback.TestTracebackException testMethod=test_no_refs_to_exception_and_traceback_objects>, 'exc_info': (<class 'ZeroDivisionError'>, ZeroDivisionError('division by zero'), <traceback object at 0x000001E0D48DE880>), 'refcnt1': 2, 'refcnt2': 3, '_': <generator object TracebackException.format at 0x000001E0D49E63C0>, 'gc': <module 'gc' (built-in)>}]

Copy link
Copy Markdown
Member Author

I went for just removing the boolean. At this point moving the header line (as an opt-in) won't make the code clearer than this.

gvanrossum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Okay, though I am curious why the original fixer didn't do it that way...

Comment thread Lib/traceback.py Outdated
if self._print_traceback_header:
if self.stack:
yield 'Traceback (most recent call last):\n'
yield from self.stack.format()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Maybe indent this line too then?

gvanrossum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Let's sit on this for a day...

gvanrossum changed the title bpo-42482: remove reference to exception traceback from TracebackExce… bpo-42482: remove reference to exc_traceback from TracebackException Nov 30, 2020
miss-islington merged commit 427613f into python:master Dec 1, 2020

Copy link
Copy Markdown
Contributor

Thanks @iritkatriel for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9.
🐍🍒⛏🤖

Copy link
Copy Markdown

GH-23578 is a backport of this pull request to the 3.9 branch.

Copy link
Copy Markdown

GH-23579 is a backport of this pull request to the 3.8 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 1, 2020
…ythonGH-23531)

(cherry picked from commit 427613f)

Co-authored-by: Irit Katriel <iritkatriel@yahoo.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 1, 2020
…ythonGH-23531)

(cherry picked from commit 427613f)

Co-authored-by: Irit Katriel <iritkatriel@yahoo.com>
miss-islington added a commit that referenced this pull request Dec 1, 2020
…H-23531)

(cherry picked from commit 427613f)

Co-authored-by: Irit Katriel <iritkatriel@yahoo.com>
miss-islington added a commit that referenced this pull request Dec 4, 2020
…ption (GH-23531) (GH-23578)

(cherry picked from commit 427613f)


Co-authored-by: Irit Katriel <iritkatriel@yahoo.com>
iritkatriel added the type-bug An unexpected behavior, bug, or error label Dec 23, 2020
iritkatriel deleted the bpo-42482 branch March 16, 2021 13:46
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type-bug An unexpected behavior, bug, or error

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants


Back | FazBrowse Home | New Git URL