-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
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
BUG: Improve ImportError message to suggest importing dependencies directly for full error details #61084
base: main
Are you sure you want to change the base?
BUG: Improve ImportError message to suggest importing dependencies directly for full error details #61084
Conversation
Hi @rhshadrach , I have implemented your suggestion from the #61030. Could you please verify if this implementation meets your expectations? Thank you! Below are example outputs before and after this change:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mroeschke - what do you think about adding tzdata
to the _hard_dependencies
on L6 here.
pandas/__init__.py
Outdated
"Unable to import required dependencies:\n" + "\n".join(_missing_dependencies) | ||
"Unable to import required dependencies:\n" | ||
+ "\n".join(_missing_dependencies) | ||
+ "\n\nTo see the full error message, " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: You can remove the +
on each line; Python will perform string concatenation automatically.
Also, I think we should remove one of the \n
here:
ImportError: Unable to import required dependencies:
dateutil: No module named 'dateutil'
To see the full error message, try importing the missing dependencies directly.
vs
ImportError: Unable to import required dependencies:
dateutil: No module named 'dateutil'
To see the full error message, try importing the missing dependencies directly.
By removing the blank line, it is more apparent to me that it's all part of the same error message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm what if we just change this to
for _dependency in _hard_dependencies:
try:
__import__(_dependency)
except ImportError as _e: # pragma: no cover
raise ImportError(f"Unable to import required dependency {_dependency} ...") from _e
I know we won't be able to collect all the missing dependencies all at once but at least we'll maintain the traceback of the failed import
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mroeschke - I'm good with your suggested approach, especially given how few hard dependencies pandas has.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Below are example outputs before and after this change:
-
before:
>>> import pandas as pd Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/home/pandas/pandas/__init__.py", line 16, in <module> raise ImportError( ImportError: Unable to import required dependencies: numpy: Error importing numpy: you should not try to import numpy from its source directory; please exit the numpy source tree, and relaunch your python interpreter from there.
-
after:
>>> import pandas as pd Traceback (most recent call last): File "/usr/local/lib/python3.10/site-packages/numpy/core/__init__.py", line 24, in <module> from . import multiarray File "/usr/local/lib/python3.10/site-packages/numpy/core/multiarray.py", line 10, in <module> from . import overrides File "/usr/local/lib/python3.10/site-packages/numpy/core/overrides.py", line 8, in <module> from numpy.core._multiarray_umath import ( ImportError: libz.so.1: cannot open shared object file: No such file or directory During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/usr/local/lib/python3.10/site-packages/numpy/__init__.py", line 130, in <module> from numpy.__config__ import show as show_config File "/usr/local/lib/python3.10/site-packages/numpy/__config__.py", line 4, in <module> from numpy.core._multiarray_umath import ( File "/usr/local/lib/python3.10/site-packages/numpy/core/__init__.py", line 50, in <module> raise ImportError(msg) ImportError: IMPORTANT: PLEASE READ THIS FOR ADVICE ON HOW TO SOLVE THIS ISSUE! Importing the numpy C-extensions failed. This error can happen for many reasons, often due to issues with your setup or how NumPy was installed. We have compiled some common reasons and troubleshooting tips at: https://numpy.org/devdocs/user/troubleshooting-importerror.html Please note and check the following: * The Python version is: Python3.10 from "/usr/local/bin/python" * The NumPy version is: "1.26.4" and make sure that they are the versions you expect. Please carefully study the documentation linked above for further help. Original error was: libz.so.1: cannot open shared object file: No such file or directory The above exception was the direct cause of the following exception: Traceback (most recent call last): File "/home/pandas/pandas/__init__.py", line 11, in <module> __import__(_dependency) File "/usr/local/lib/python3.10/site-packages/numpy/__init__.py", line 135, in <module> raise ImportError(msg) from e ImportError: Error importing numpy: you should not try to import numpy from its source directory; please exit the numpy source tree, and relaunch your python interpreter from there. The above exception was the direct cause of the following exception: Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/home/pandas/pandas/__init__.py", line 13, in <module> raise ImportError(f"Unable to import required dependency {_dependency} ...") from _e ImportError: Unable to import required dependency numpy ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm what if we just change this to
for _dependency in _hard_dependencies: try: __import__(_dependency) except ImportError as _e: # pragma: no cover raise ImportError(f"Unable to import required dependency {_dependency} ...") from _eI know we won't be able to collect all the missing dependencies all at once but at least we'll maintain the traceback of the failed import
This change might fail in the current test case:
pandas/pandas/tests/test_downstream.py
Lines 222 to 223 in 89bc204
for name in ["numpy", "dateutil"]: | |
assert name in output |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @rhshadrach and @mroeschke , since the original test_missing_required_dependency
failed after applying the change, I attempted to rewrite it in 1826c0 to ensure that each missing dependency raises an exception. I'm not sure if this rewrite aligns with expectations—any feedback would be appreciated. Thanks! 🙏
Yeah we should probably add that too. |
@mroeschke For |
Please create a separate issue for this |
rf"Command '\['{pyexe}', '-sSE', '-c', 'import pandas'\]' " | ||
"returned non-zero exit status 1." | ||
) | ||
@pytest.mark.parametrize("dependency", ["numpy", "dateutil"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we monkeypatch _hard_dependencies
with a fake module and test that instead? I'm worried about this test having side effects by modifying sys.modules
doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.