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

Fix exception chaining in test/ #44193

Closed

Conversation

akihironitta
Copy link
Contributor

@akihironitta akihironitta commented Sep 4, 2020

Motivation

This PR fixes #43770 and is the continuation of #43836.

Description of the change

This PR fixes exception chaining only in files under test/ where appropriate.
To fix exception chaining, I used either:

  1. raise new_exception from old_exception where new_exception itself seems not descriptive enough to debug or old_exception delivers valuable information.
  2. raise new_exception from None where raising both of new_exception and old_exception seems a bit noisy and redundant.

List of lines containing raise in except clause:

I wrote this simple script using ast to list lines where raiseing in except clause.

Sorry, something went wrong.

@akihironitta akihironitta changed the title Fix exception chaining in test/ [WIP] Fix exception chaining in test/ Sep 4, 2020
@dr-ci
Copy link

dr-ci bot commented Sep 4, 2020

💊 CI failures summary and remediations

As of commit 00488ae (more details on the Dr. CI page):


💚 💚 Looks good so far! There are no failures yet. 💚 💚


This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 14 times.

@akihironitta akihironitta changed the title [WIP] Fix exception chaining in test/ Fix exception chaining in test/ Sep 4, 2020
@akihironitta akihironitta marked this pull request as ready for review September 4, 2020 15:55
@akihironitta
Copy link
Contributor Author

@malfet Could you review the changes as you reviewed the previous PR #43836 related to this PR?

@codecov
Copy link

codecov bot commented Sep 14, 2020

Codecov Report

Merging #44193 into master will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #44193   +/-   ##
=======================================
  Coverage   67.98%   67.98%           
=======================================
  Files         384      384           
  Lines       49567    49567           
=======================================
+ Hits        33697    33699    +2     
+ Misses      15870    15868    -2     
Impacted Files Coverage Δ
torch/testing/_internal/common_utils.py 77.41% <0.00%> (+0.18%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8daaa3b...00488ae. Read the comment docs.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@malfet has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@malfet merged this pull request in 8494967.

xuzhao9 pushed a commit that referenced this pull request Sep 18, 2020
Summary:
## Motivation
This PR fixes #43770 and is the continuation of #43836.

## Description of the change
This PR fixes exception chaining only in files under `test/` where appropriate.
To fix exception chaining, I used either:
1. `raise new_exception from old_exception` where `new_exception` itself seems not descriptive enough to debug or `old_exception` delivers valuable information.
2. `raise new_exception from None` where raising both of `new_exception` and `old_exception` seems a bit noisy and redundant.

## List of lines containing `raise` in `except` clause:
I wrote [this simple script](https://gist.github.com/akihironitta/4223c1b32404b36c1b349d70c4c93b4d) using [ast](https://docs.python.org/3.8/library/ast.html#module-ast) to list lines where `raise`ing in `except` clause.

- [x] https://github.com/pytorch/pytorch/blob/f8f35fddd4b2bd788953d6d6ccfadf690b73a3e8/test/test_cpp_extensions_aot.py#L16
- [x] https://github.com/pytorch/pytorch/blob/f8f35fddd4b2bd788953d6d6ccfadf690b73a3e8/test/test_jit.py#L2503
- [x] https://github.com/pytorch/pytorch/blob/f8f35fddd4b2bd788953d6d6ccfadf690b73a3e8/test/onnx/model_defs/word_language_model.py#L22
- [x] https://github.com/pytorch/pytorch/blob/f8f35fddd4b2bd788953d6d6ccfadf690b73a3e8/test/onnx/verify.py#L73
- [x] https://github.com/pytorch/pytorch/blob/f8f35fddd4b2bd788953d6d6ccfadf690b73a3e8/test/onnx/verify.py#L110
- [x] https://github.com/pytorch/pytorch/blob/f8f35fddd4b2bd788953d6d6ccfadf690b73a3e8/test/onnx/test_verify.py#L31
- [x] https://github.com/pytorch/pytorch/blob/f8f35fddd4b2bd788953d6d6ccfadf690b73a3e8/test/distributed/test_c10d.py#L255
- [x] https://github.com/pytorch/pytorch/blob/f8f35fddd4b2bd788953d6d6ccfadf690b73a3e8/test/distributed/test_c10d.py#L2992
- [x] https://github.com/pytorch/pytorch/blob/f8f35fddd4b2bd788953d6d6ccfadf690b73a3e8/test/distributed/test_c10d.py#L3025
- [x] https://github.com/pytorch/pytorch/blob/f8f35fddd4b2bd788953d6d6ccfadf690b73a3e8/test/distributed/test_c10d.py#L3712
- [x] https://github.com/pytorch/pytorch/blob/f8f35fddd4b2bd788953d6d6ccfadf690b73a3e8/test/distributed/test_distributed.py#L3180
- [x] https://github.com/pytorch/pytorch/blob/f8f35fddd4b2bd788953d6d6ccfadf690b73a3e8/test/distributed/test_distributed.py#L3198
- [x] https://github.com/pytorch/pytorch/blob/f8f35fddd4b2bd788953d6d6ccfadf690b73a3e8/test/distributed/test_data_parallel.py#L752
- [x] https://github.com/pytorch/pytorch/blob/f8f35fddd4b2bd788953d6d6ccfadf690b73a3e8/test/distributed/test_data_parallel.py#L776
- [x] https://github.com/pytorch/pytorch/blob/f8f35fddd4b2bd788953d6d6ccfadf690b73a3e8/test/test_type_hints.py#L151
- [x] https://github.com/pytorch/pytorch/blob/f8f35fddd4b2bd788953d6d6ccfadf690b73a3e8/test/test_jit_fuser.py#L771
- [x] https://github.com/pytorch/pytorch/blob/f8f35fddd4b2bd788953d6d6ccfadf690b73a3e8/test/test_jit_fuser.py#L773
- [x] https://github.com/pytorch/pytorch/blob/f8f35fddd4b2bd788953d6d6ccfadf690b73a3e8/test/test_dispatch.py#L105
- [x] https://github.com/pytorch/pytorch/blob/f8f35fddd4b2bd788953d6d6ccfadf690b73a3e8/test/test_distributions.py#L4738
- [x] https://github.com/pytorch/pytorch/blob/f8f35fddd4b2bd788953d6d6ccfadf690b73a3e8/test/test_nn.py#L9824
- [x] https://github.com/pytorch/pytorch/blob/f8f35fddd4b2bd788953d6d6ccfadf690b73a3e8/test/test_namedtensor.py#L843
- [x] https://github.com/pytorch/pytorch/blob/f8f35fddd4b2bd788953d6d6ccfadf690b73a3e8/test/test_jit_fuser_te.py#L875
- [x] https://github.com/pytorch/pytorch/blob/f8f35fddd4b2bd788953d6d6ccfadf690b73a3e8/test/test_jit_fuser_te.py#L877
- [x] https://github.com/pytorch/pytorch/blob/f8f35fddd4b2bd788953d6d6ccfadf690b73a3e8/test/test_dataloader.py#L31
- [x] https://github.com/pytorch/pytorch/blob/f8f35fddd4b2bd788953d6d6ccfadf690b73a3e8/test/test_dataloader.py#L43
- [x] https://github.com/pytorch/pytorch/blob/f8f35fddd4b2bd788953d6d6ccfadf690b73a3e8/test/test_dataloader.py#L365
- [x] https://github.com/pytorch/pytorch/blob/f8f35fddd4b2bd788953d6d6ccfadf690b73a3e8/test/test_dataloader.py#L391

Pull Request resolved: #44193

Reviewed By: albanD

Differential Revision: D23681529

Pulled By: malfet

fbshipit-source-id: 7c2256ff17334625081137b35baeb816c1e53e0b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix exception chaining all over the codebase
5 participants