ENH: don't drop ImportError tracebacks for lazily-loaded optional dependencies #4226
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
neutrinoceros
force-pushed
the
import_error_notes
branch
from
a4fd03b to
a10054c
Compare
neutrinoceros
force-pushed
the
import_error_notes
branch
from
a10054c to
4bc7b83
Compare
chummels
approved these changes
Nov 24, 2022
chrishavlin
approved these changes
Dec 6, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Summary
Closes #4224
The original issue I reported there had nothing to do with yt, but it took me a couple hours to even find out that yt was indeed not the culprit, because the initially insightful error message was completely replaced by a less informative one. This patch makes sure that, failing to import an optional dependency, we always report the original error (because as we learned, the problem might not be that the package isn't installed, it can also be broken). That is, alongside with additional insight specific to yt.
The implementation is more elegant in Python 3.11 thanks to the newly introduced exception notes, but I think I was able to make the behaviour consistent with older versions too.
I intend this patch as backward compatible in case anyone downstream relies on the
NotAModuleclass, but I don't think it needs to be backported