The Wayback Machine - https://web.archive.org/web/20200717004419/https://github.com/numpy/numpy/issues/15986
Skip to content

/ numpy

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

Jump to bottom

Chain exceptions where appropriate #15986

Open
eric-wieser opened this issue Apr 15, 2020 · 6 comments
Open

Chain exceptions where appropriate #15986

eric-wieser opened this issue Apr 15, 2020 · 6 comments
Labels
good first issue

Comments

@eric-wieser
Copy link
Member

@eric-wieser eric-wieser commented Apr 15, 2020 •
edited

Picking up from #15731, there are many places in numpy where we do something like:

try:
    something_which_throws_typeError
except TypeError:
    raise ValueError("a clearer explanation of what went wrong")

It would produce a marginally clearer error if we could change these to use traceback chaining. Our two options are either:

  1. The inner error provides valuable information that is not present in the outer error. We should use from e. 
    try:
    something_which_throws_typeError
except TypeError as e:
    raise ValueError("a clearer explanation of what went wrong") from e
  2. The inner error provides no valuable information that is not present in the outer error. We should use from None: 
    try:
    some_dict[name]
except KeyError:
    raise ValueError("The name %s is not supported") from None

An example of such a change would be this line of this otherwise-discarded patch.

For now, I would recommend we only apply this to cases where the exception is thrown unconditionally, as other cases can be more nuanced.
@eric-wieser eric-wieser mentioned this issue Apr 16, 2020
Closed
@JanukanS
Copy link

@JanukanS JanukanS commented Apr 17, 2020

Hi, could I attempt to fix this issue. I've read through the contribution guide
@seberg
Copy link
Member

@seberg seberg commented Apr 17, 2020

Sure @JanukanS, thanks. We don't usually assign issues. So you can search for these type of calls and simply open a PR with suggested changes linking this issue.
@keremh
Copy link
Contributor

@keremh keremh commented Apr 18, 2020

Hi @seberg
I determined the part to be changed, can i work on this issue too?
This was referenced Apr 22, 2020
Merged
Merged
This was referenced Apr 24, 2020
Closed
Merged
@keremh keremh mentioned this issue Apr 24, 2020
Merged
@ChrisAHolland ChrisAHolland mentioned this issue Apr 24, 2020
Merged
@keremh keremh mentioned this issue Apr 30, 2020
Merged
@bartosz-grabowski bartosz-grabowski mentioned this issue May 12, 2020
Merged
@smitgajjar smitgajjar mentioned this issue May 12, 2020
Closed
seberg pushed a commit that referenced this issue May 14, 2020
@bartosz-grabowski @eric-wieser @rossbar
MAINT: Chain exceptions and use NameError in np.bmat (#16215)
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
a813a5f 
This solution is related to the issue #15986. I also made a change to the newer string formatting.
Uses NameError, which prints nicer, and is actually the more correct error type.

Co-authored-by: Eric Wieser <[email protected]>
Co-authored-by: Ross Barnowski <[email protected]>
@lkdmttg7 lkdmttg7 mentioned this issue May 15, 2020
Merged
@zaak71 zaak71 mentioned this issue May 28, 2020
Merged
anirudh2290 added a commit to anirudh2290/numpy that referenced this issue May 28, 2020
@bartosz-grabowski @eric-wieser
@rossbar @anirudh2290
MAINT: Chain exceptions and use NameError in np.bmat (numpy#16215)
5959c53 
This solution is related to the issue numpy#15986. I also made a change to the newer string formatting.
Uses NameError, which prints nicer, and is actually the more correct error type.

Co-authored-by: Eric Wieser <[email protected]>
Co-authored-by: Ross Barnowski <[email protected]>
dojafrat added a commit to dojafrat/numpy that referenced this issue Jun 14, 2020
@bartosz-grabowski @eric-wieser
@rossbar @dojafrat
MAINT: Chain exceptions and use NameError in np.bmat (numpy#16215)
024230f 
This solution is related to the issue numpy#15986. I also made a change to the newer string formatting.
Uses NameError, which prints nicer, and is actually the more correct error type.

Co-authored-by: Eric Wieser <[email protected]>
Co-authored-by: Ross Barnowski <[email protected]>
This was referenced Jun 23, 2020
Merged
Merged
@ayao451
Copy link

@ayao451 ayao451 commented Jun 30, 2020

Can I work on this issue? It is my first issue and I want to give it a try!
seberg pushed a commit that referenced this issue Jun 30, 2020
@keremh @rossbar
MAINT: Chain exceptions in npyio.py (gh16121)
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
f38073b 
this solution is related to the following issue #15986


Co-authored-by: Ross Barnowski <[email protected]>
@rossbar
Copy link
Contributor

@rossbar rossbar commented Jul 1, 2020

@ayao451 please feel free. A good place to start might be by checking the PRs linked in this issue: exception chaining has been added in quite a few places, and a few other instances have been identified as better off unchanged (e.g. nested exceptions in the test suite).
@abhinav-thimma abhinav-thimma mentioned this issue Jul 4, 2020
Merged
@sarthakchaudhary13 sarthakchaudhary13 mentioned this issue Jul 6, 2020
Merged
@rossbar
Copy link
Contributor

@rossbar rossbar commented Jul 12, 2020

A lot of these instances have been fixed (thanks to all the contributors for this!) It would be nice to have a "definition of done" for this issue if anyone wants to do a little forensics to see what's already been tackled, what's been reviewed and determined better left as-is, and what still remains to be done.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
@seberg @eric-wieser @rossbar @keremh @ayao451 @JanukanS
You can’t perform that action at this time.