-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Improve error message when scipy is missing for NDPointIndex #11085
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
Improve error message when scipy is missing for NDPointIndex #11085
Conversation
|
Thank you for opening this pull request! It may take us a few days to respond here, so thank you for being patient. |
|
thanks for the PR, @Sakshee-D. This looks good to me, but we might need a test? This might require more changes, though: right now, the dedicated test module ( |
|
Thanks for the suggestion. I’ll include this in the PR so you can review it there and suggest any changes if needed. |
| from xarray.indexes import NDPointIndex | ||
|
|
||
|
|
||
| def test_ndpointindex_missing_scipy(monkeypatch): |
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.
instead of the monkey patching it should be enough to check on the environments that don't install scipy (also needs from xarray.tests import has_scipy):
| def test_ndpointindex_missing_scipy(monkeypatch): | |
| @pytest.mark.skipif(has_scipy) | |
| def test_ndpointindex_missing_scipy(): |
|
Thanks for pointing that out . |
This PR improves the error message raised when using
NDPointIndexwithoutscipyinstalled.Previously, the missing dependency error was raised indirectly during index creation. This change wraps the
scipyimport inScipyKDTreeAdapterand raises a more actionableImportErrorexplaining thatscipyis required.Manual testing
scipyis not installedscipyis installedRelated to #11047