You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This is a Meta issue to encourage contributors new and well-seasoned to consider where Astropy can use Exceptions with Notes.
One good category is wherever we catch an error, modify the message, and re-raise the same error, e.g. in #16878.
Describe the desired outcome
Please open PRs making our Exception messages more useful!
Additional context
EDIT: Would be much easier to wait for the following bumps first:
requires-python = ">=3.11"
pytest>=8.0
This change might also break downstream testing for our exception message with rigid match. Actual impact is unknown.
So, is the ask here to use "notes" or to use "raise from"?
I think I am going to remove "good first issue". We need to actually pinpoint what code can be improved with this ask before asking new contributors to implement. Thanks for your understanding.
This might break downstream if they actually look for specific error messages, so we should probably consider the cost/benefit on a case by case basis.
This shouldn't change the starts of any messages as notes are appended after the error message.
Indeed, this is why I liked it so much (even if AttributeError is perhaps not the most logical).
But #16884 made clear that as long as we do not require pytest >= 8.0, one has to be careful, since the notes do not get added to the exception message that pytest sees.
I forgot about this, but note that we have a helper function astropy.utils.exceptions._add_note_to_exception whose whole purpose is to handle older Python versions transparently.
I forgot about this, but note that we have a helper function astropy.utils.exceptions._add_note_to_exception whose whole purpose is to handle older Python versions transparently.
Yikes! Though hopefully it will soon become irrelevant as we drop python 3.10 -- #16900
Activity
neutrinoceros commentedon Aug 26, 2024
Exception.add_noteis new in Python 3.11 IIRC, and we still support 3.10, so it might be a bit early for PRs, right ?nstarman commentedon Aug 26, 2024
sys.version_infoa la #16878pllim commentedon Aug 26, 2024
So, is the ask here to use "notes" or to use "raise from"?
I think I am going to remove "good first issue". We need to actually pinpoint what code can be improved with this ask before asking new contributors to implement. Thanks for your understanding.
pllim commentedon Aug 26, 2024
This might break downstream if they actually look for specific error messages, so we should probably consider the cost/benefit on a case by case basis.
nstarman commentedon Aug 26, 2024
The former. Add notes.
This shouldn't change the starts of any messages as notes are appended after the error message.
mhvk commentedon Aug 26, 2024
Indeed, this is why I liked it so much (even if
AttributeErroris perhaps not the most logical).But #16884 made clear that as long as we do not require
pytest >= 8.0, one has to be careful, since the notes do not get added to the exception message thatpytestsees.pllim commentedon Aug 26, 2024
Based on the conversation so far, I added stuff to "additional details" section in the original post above. Hope you don't mind!
neutrinoceros commentedon Aug 29, 2024
I forgot about this, but note that we have a helper function
astropy.utils.exceptions._add_note_to_exceptionwhose whole purpose is to handle older Python versions transparently.mhvk commentedon Aug 29, 2024
Yikes! Though hopefully it will soon become irrelevant as we drop python 3.10 -- #16900
neutrinoceros commentedon Aug 29, 2024
Yup, I'm removing it in #16903
pllim commentedon Aug 29, 2024
Just for my own future reference,
astropy.utils.exceptions._add_note_to_exceptionwas added in #16346 that was also milestoned to v7.0