- Sponsor
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[traceback] Add traceback frame opt-out via a _rich_traceback_omit = True
machinery
#2226
[traceback] Add traceback frame opt-out via a _rich_traceback_omit = True
machinery
#2226
Conversation
# fmt: off | ||
[True, 3, ["test_rich_traceback_omit_optional_local_flag", "level1", "level3"]], | ||
[False, 4, ["test_rich_traceback_omit_optional_local_flag", "level1", "level2", "level3"]], | ||
# fmt: on |
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.
I added these # fmt:
annotations just to avoid Black changing the second test case so that it spawns across multiple lines when the 1st one doesn't, as they are functionally the same thing
But happy to let Black format everything and remove those if you prefer, of course 🙂
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.
I like this. Black completely ruins the readability of some parametrised tests
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.
Fine by me!
rich/traceback.py
Outdated
@@ -367,6 +367,8 @@ def safe_str(_object: Any) -> str: | |||
if filename and not filename.startswith("<"): | |||
if not os.path.isabs(filename): | |||
filename = os.path.join(_IMPORT_CWD, filename) | |||
if frame_summary.f_locals.get("_rich_traceback_omit", False) is True: |
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.
The _rich_traceback_guard
mechanism is triggered by just the presence of the local (i.e. doesn't care about the value). I think this should be consistent.
On reflection I think we should look at the value to make it easier to add a condition. The default should be False
if the local isn't defined.
Something like this:
if getattr(frame_summary.f_locals, "_rich_traceback_omit", False):
...
The "_rich_traceback_guard"
should also be updated in the same way.
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.
I see! However, f_locals
being a Python dictionary it doesn't support the getattr
protocol if I'm not wrong?
But I updated the _rich_traceback_guard
management so that both look the same. Also, they are both activated as soon as the value is defined in the locals and its value is true-ish value, rather than strict True
🙂
--> done there: f0ca6ff
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.
My bad, should of course be .get
.
Type of changes
Checklist
Description
closes #2207