Explicitly chain exceptions with raise ... from ...

[akihironitta]

Before submitting

• Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
• Did you read the contributor guideline, Pull Request section?
• Did you make sure to update the docs?
• Did you write any new necessary tests?

What does this PR do?

I'd like to suggest a fix in the way that Python 3's exception chaining is used. I recently submitted a PR and got it merged into PyTorchLightning at Lightning-AI/pytorch-lightning#3750, and I'd like to apply the fix here as well.

As described in detail in this article, exception chaining (PEP 3134) can be used to make exceptions more user-friendly, and in that case, the syntax raise ... from ... needs to be used.

When raise ... from ... is used, there will be a line saying The above exception was the direct cause of the following exception: between tracebacks. However, when implicitly chaining exceptions (meaning when raise ... from ... is not used), the message will be During handling of the above exception, another exception occurred: which can confuse users.

Specifically, the following should be used in order to explicitly chain exceptions:

try:
    something_which_raises_OldError
except OldError as e:
    raise NewError("A more user-friendly exception message.") from e 

instead of:

try:
    something_which_raises_OldError
except OldError:
    raise NewError("A more user-friendly exception message.")

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃

[codecov]

Codecov Report

Merging #261 into master will increase coverage by 0.34%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #261      +/-   ##
==========================================
+ Coverage   83.25%   83.59%   +0.34%     
==========================================
  Files          89       89              
  Lines        4664     4664              
==========================================
+ Hits         3883     3899      +16     
+ Misses        781      765      -16     

Flag	Coverage Δ
#cpu	25.25% <0.00%> (ø)
#pytest	25.25% <0.00%> (ø)
#unittests	83.04% <0.00%> (+0.34%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pl_bolts/callbacks/vision/confused_logit.py	100.00% <ø> (ø)
pl_bolts/datamodules/base_dataset.py	95.45% <0.00%> (ø)
pl_bolts/datamodules/mnist_dataset.py	42.85% <0.00%> (ø)
pl_bolts/models/regression/linear_regression.py	98.50% <0.00%> (ø)
pl_bolts/models/regression/logistic_regression.py	93.15% <0.00%> (ø)
pl_bolts/models/self_supervised/cpc/cpc_module.py	21.09% <0.00%> (ø)
...ts/models/autoencoders/basic_ae/basic_ae_module.py	87.83% <0.00%> (+9.45%)	⬆️
.../models/autoencoders/basic_vae/basic_vae_module.py	91.48% <0.00%> (+9.57%)	⬆️

---

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 07e3376...72b65d9. Read the comment docs.

[mergify]

This pull request is now in conflict... :(

[Borda]

@akihironitta update/rebased, mind add the same to all ImportExceptions?

[akihironitta]

@Borda Thanks for your reponse! It looks good to go! There are warn() in except ModuleNotFoundError clauses, but those warn()ings are not exceptions so they don't need to be chained with raise ... from ..., I believe.