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

bpo-45607: Make it possible to enrich exception displays via setting their __note__ field by iritkatriel · Pull Request #29880 · python/cpython · GitHub

/ cpython Public

bpo-45607: Make it possible to enrich exception displays via setting their __note__ field - #29880

Merged
iritkatriel merged 19 commits into
python:mainfrom
iritkatriel:bpo-45607-exception-note
Dec 3, 2021
Merged

bpo-45607: Make it possible to enrich exception displays via setting their __note__ field#29880
iritkatriel merged 19 commits into
python:mainfrom
iritkatriel:bpo-45607-exception-note

Conversation

iritkatriel commented Dec 1, 2021
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

Copy link
Copy Markdown
Member Author

Should I add PyException_GetNote and PyException_SetNote to the c api?

erlend-aasland left a comment

Copy link
Copy Markdown
Contributor

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

Nice feature! LGTM. I found one path not covered by the test suite. I of course also left some pedantic PEP-7 comments 🤓

(BTW, I still find the error handling in pythonrun.c hard to read... and it generates so many indent levels 😕)

Comment thread Objects/exceptions.c Outdated
Comment thread Objects/exceptions.c Outdated
Comment thread Objects/exceptions.c Outdated
Comment thread Lib/traceback.py Outdated
Comment thread Doc/library/exceptions.rst Outdated
Comment thread Objects/exceptions.c
iritkatriel and others added 5 commits December 2, 2021 10:07
Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>
Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>
Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>
Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>

Copy link
Copy Markdown
Member Author

Thanks.

(BTW, I still find the error handling in pythonrun.c hard to read... and it generates so many indent levels 😕)

I'm working on it. I made a patch that changed so much it would take a year to review. I'll break it up.

Comment thread Objects/exceptions.c Outdated
Comment thread Objects/exceptions.c Outdated

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

Nice! LGTM, but please fix the issues Erlend brought up.

Copy link
Copy Markdown
Member Author

Nice! LGTM, but please fix the issues Erlend brought up.

I did all of them except the test coverage for the error when passing NULL to the setter. This would only come into play if I add PyException_GetNote and PyException_SetNote to the C api. I think we need those, right?

Copy link
Copy Markdown
Member

I did all of them except the test coverage for the error when passing NULL to the setter. This would only come into play if I add PyException_GetNote and PyException_SetNote to the C api. I think we need those, right?

I think we should not add to the C API. If people want to access the note from C, they can use the __note__ attribute (like you do in the exception-printing C code). Even so, what happens when you try del err.__note__?

Copy link
Copy Markdown
Member Author

Ah yes, very good:

>>> del e.__note__
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: __note__ may not be deleted

gvanrossum commented Dec 3, 2021 via email

Copy link
Copy Markdown
Member

Copy link
Copy Markdown
Member Author

That behavior is fine. So do you have a unit test for that? If not, it should be easy to add one.

I don’t, I’ll add one.

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

+1

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants


Back | FazBrowse Home | New Git URL