Skip to content
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

Use exception chaining when re-raising numpy exceptions within BoundedArray.__init__ #6

Closed
wants to merge 1 commit into from

Conversation

cool-RR
Copy link

@cool-RR cool-RR commented Mar 11, 2021

I recently went over Matplotlib, Pandas and NumPy, fixing a small mistake in the way that Python 3's exception chaining is used. I found the same mistake here in two places, and fixed it in this PR.

The mistake is this: An exception is being caught and replaced with a more user-friendly error. In these cases the syntax raise new_error from old_error needs to be used.

Python 3's exception chaining means it shows not only the traceback of the current exception, but that of the original exception (and possibly more.) This is regardless of raise from. The usage of raise from tells Python to put a more accurate message between the tracebacks. Instead of this:

During handling of the above exception, another exception occurred:

You'll get this:

The above exception was the direct cause of the following exception:

The first is inaccurate, because it signifies a bug in the exception-handling code itself, which is a separate situation than wrapping an exception.

Let me know what you think!

@google-cla google-cla bot added the cla: yes label Mar 11, 2021
Copy link
Collaborator

@alimuldal alimuldal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - this is basically a hangover from the fact that we needed to support Python 2 until recently.

I don't think there's any need to include the message from the original exception in the new exception message, since the original exception will still visible in the traceback. I'd just do:

except ValueError as numpy_exception:
  raise ValueError('`minimum` is incompatible with `shape`') from numpy_exception

@cool-RR
Copy link
Author

cool-RR commented Mar 12, 2021

Agreed, fixed.

@alimuldal alimuldal changed the title Fix exception causes in BoundedArray.__init__ Use exception chaining when re-raising numpy exceptions within BoundedArray.__init__ Mar 12, 2021
@alimuldal
Copy link
Collaborator

Merged in I9d16586a559d938d608afcaf06500ddc92adb209. Thanks!

@alimuldal alimuldal closed this Mar 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants