-
-
Notifications
You must be signed in to change notification settings - Fork 31.4k
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
bpo-42877: add the 'compact' param to TracebackException's __init__ #24179
Conversation
I don't know whether a change like this needs news. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I don't think you can make this change at all -- __cause__
and __context__
are documented and hence public APIs.
Maybe a different way to save space/effort is to set __context__
to None if there is a __cause__
-- but even that seems to violate the documented value for this attribute.
I see. I didn't realise all these fields are in the doc. This class is probably mostly used from print_exception etc, we could add this as an opt-in optimisation to use from there. Is that worth doing? |
I don't know -- I guess you ran into this when solving the recursion error issue? I guess you could add a new keyword parameter |
I'd pass in a bool (like compact or save_space) and leave the logic of whether to skip internet he class itself (note that there is also Yes, this came up re the recursion issue. I'm still not sure what to do about that. |
Since the latter is set on the original exception we can't change it. Agreed to put as much logic in the constructor. |
(This would also require a news item.) |
Yes, and some new tests. Since this won't simplify the code, it won't help deal with the recursion issue so it can come after PR 24158, which I've now refactored to use the fact that the chained exceptions are a list and not a tree. |
e4e4eab
to
f9e2a98
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like you have a merge conflict due to your own #24158.
Doc/library/traceback.rst
Outdated
*capture_locals* are as for the :class:`StackSummary` class. | ||
*capture_locals* are as for the :class:`StackSummary` class. If *compact* | ||
is true, only data that is required for :method:`format` and | ||
:method:`format_exception_only` is saved in the class attributes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe clarify that this suppresses context if cause is not None?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, though I'll avoid "suppresses" so as not to confuse with __suppress_context__
which is applied in format().
I also don't need to mention format_exception_only because it's called from format.
Lib/traceback.py
Outdated
and id(exc_value.__context__) not in _seen): | ||
|
||
self.__suppress_context__ = \ | ||
exc_value.__suppress_context__ if exc_value else False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exc_value.__suppress_context__ if exc_value else False | |
exc_value.__suppress_context__ if exc_value is not None else False |
(Some joker could add a __bool__
method to their exception that sometimes makes it falsey.)
…d use it to reduce the time and memory taken up by several of traceback's module-level functions
f9e2a98
to
f0301fa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup!
@gvanrossum: Please replace |
…ython#24179) Use it to reduce the time and memory taken up by several of traceback's module-level functions.
…re is cause
https://bugs.python.org/issue42877