Skip to content

Use Exceptions with Notes #16879

Open
Open

Description

@nstarman

What is the problem this feature will solve?

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.

Activity

  1. neutrinoceros commented on Aug 26, 2024

    @neutrinoceros
    Contributor

    Exception.add_note is new in Python 3.11 IIRC, and we still support 3.10, so it might be a bit early for PRs, right ?

  2. nstarman commented on Aug 26, 2024

    @nstarman
    MemberAuthor

    sys.version_info a la #16878

  3. pllim commented on Aug 26, 2024

    @pllim
    Member

    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.

  4. added
    API changePRs and issues that change an existing API, possibly requiring a deprecation period
    and removed
    💤 good first issueIssues that are well-suited for new contributors
    on Aug 26, 2024
  5. pllim commented on Aug 26, 2024

    @pllim
    Member

    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.

  6. nstarman commented on Aug 26, 2024

    @nstarman
    MemberAuthor

    use "notes" or to use "raise from"

    The former. Add notes.
    This shouldn't change the starts of any messages as notes are appended after the error message.

  7. mhvk commented on Aug 26, 2024

    @mhvk
    Contributor

    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.

  8. pllim commented on Aug 26, 2024

    @pllim
    Member

    Based on the conversation so far, I added stuff to "additional details" section in the original post above. Hope you don't mind!

  9. neutrinoceros commented on Aug 29, 2024

    @neutrinoceros
    Contributor

    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.

  10. mhvk commented on Aug 29, 2024

    @mhvk
    Contributor

    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

  11. neutrinoceros commented on Aug 29, 2024

    @neutrinoceros
    Contributor

    Yup, I'm removing it in #16903

  12. pllim commented on Aug 29, 2024

    @pllim
    Member

    Just for my own future reference, astropy.utils.exceptions._add_note_to_exception was added in #16346 that was also milestoned to v7.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    API changePRs and issues that change an existing API, possibly requiring a deprecation periodFeature RequestPython 3.11

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions