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

more friendly error message in case no chunk manager is available #9676

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
2 changes: 2 additions & 0 deletions doc/whats-new.rst
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ Bug fixes
By `Stephan Hoyer <https://github.com/shoyer>`_.
- Fix regression in the interoperability of :py:meth:`DataArray.polyfit` and :py:meth:`xr.polyval` for date-time coordinates. (:pull:`9691`).
By `Pascal Bourgault <https://github.com/aulemahal>`_.
- Improve the error message raised when using chunked-array methods if no chunk manager is available (:pull:`9676`)
By `Justus Magin <https://github.com/keewis>`_.
- Fix CF decoding of ``grid_mapping`` to allow all possible formats, add tests (:issue:`9761`, :pull:`9765`).
By `Kai Mühlbauer <https://github.com/kmuehlbauer>`_.
- Add ``User-Agent`` to request-headers when retrieving tutorial data (:issue:`9774`, :pull:`9782`)
Expand Down
4 changes: 4 additions & 0 deletions xarray/namedarray/parallelcompat.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@ def guess_chunkmanager(
"""

chunkmanagers = list_chunkmanagers()
if len(chunkmanagers) == 0:
raise ImportError(
"no chunk managers available. Try installing `dask` or another package that provides a chunk manager."
)

if manager is None:
if len(chunkmanagers) == 1:
Expand Down
14 changes: 8 additions & 6 deletions xarray/tests/test_parallelcompat.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
list_chunkmanagers,
load_chunkmanagers,
)
from xarray.tests import has_dask, requires_dask
from xarray.tests import requires_dask


class DummyChunkedArray(np.ndarray):
Expand Down Expand Up @@ -158,18 +158,20 @@
chunkmanager = guess_chunkmanager(None)
assert isinstance(chunkmanager, DummyChunkManager)

def test_fail_on_nonexistent_chunkmanager(self) -> None:
with pytest.raises(ValueError, match="unrecognized chunk manager foo"):
def test_fail_on_nonexistent_chunkmanager(
self, register_dummy_chunkmanager
) -> None:
with pytest.raises(ImportError, match="unrecognized chunk manager foo"):
Copy link
Collaborator Author

@keewis keewis Nov 18, 2024

Choose a reason for hiding this comment

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

there might be two reasons why this fails: one, there's a typo, in which case I'd say it would be better to raise a ValueError (the issue is the caller's input), while the second reason is indeed that the library that provides the requested chunk manager was not installed or fails to import.

In the case when there's no chunk manager at all, we know that the user needs to install a library and I can understand using a ImportError. However, in the case where there's at least one chunk manager I don't think we can figure out whether the issue was a user error or a missing library (at least, without maintaining a list of known chunk managers and suggesting sufficiently "close" names), so I think this should still be expecting a ValueError:

Suggested change
with pytest.raises(ImportError, match="unrecognized chunk manager foo"):
with pytest.raises(ValueError, match="unrecognized chunk manager foo"):

Copy link
Member

Choose a reason for hiding this comment

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

You raise a good point about ambiguity, but I also think we could reasonably just say (imprecisely) "with this user input were unable to import a necessary library, so there's still an ImportError underneath", which then has the advantage of consistency of error types for users.

guess_chunkmanager("foo")

Check failure on line 165 in xarray/tests/test_parallelcompat.py

View workflow job for this annotation

GitHub Actions / ubuntu-latest py3.10

TestGetChunkManager.test_fail_on_nonexistent_chunkmanager ValueError: unrecognized chunk manager foo - must be one of: ['dummy', 'dask']

Check failure on line 165 in xarray/tests/test_parallelcompat.py

View workflow job for this annotation

GitHub Actions / ubuntu-latest py3.12

TestGetChunkManager.test_fail_on_nonexistent_chunkmanager ValueError: unrecognized chunk manager foo - must be one of: ['dummy', 'dask']

Check failure on line 165 in xarray/tests/test_parallelcompat.py

View workflow job for this annotation

GitHub Actions / macos-latest py3.10

TestGetChunkManager.test_fail_on_nonexistent_chunkmanager ValueError: unrecognized chunk manager foo - must be one of: ['dummy', 'dask']

Check failure on line 165 in xarray/tests/test_parallelcompat.py

View workflow job for this annotation

GitHub Actions / macos-latest py3.12

TestGetChunkManager.test_fail_on_nonexistent_chunkmanager ValueError: unrecognized chunk manager foo - must be one of: ['dummy', 'dask']

Check failure on line 165 in xarray/tests/test_parallelcompat.py

View workflow job for this annotation

GitHub Actions / ubuntu-latest py3.10 bare-minimum

TestGetChunkManager.test_fail_on_nonexistent_chunkmanager ValueError: unrecognized chunk manager foo - must be one of: ['dummy']

Check failure on line 165 in xarray/tests/test_parallelcompat.py

View workflow job for this annotation

GitHub Actions / ubuntu-latest py3.10 min-all-deps

TestGetChunkManager.test_fail_on_nonexistent_chunkmanager ValueError: unrecognized chunk manager foo - must be one of: ['dummy', 'dask']

Check failure on line 165 in xarray/tests/test_parallelcompat.py

View workflow job for this annotation

GitHub Actions / ubuntu-latest py3.12 all-but-numba

TestGetChunkManager.test_fail_on_nonexistent_chunkmanager ValueError: unrecognized chunk manager foo - must be one of: ['dummy', 'dask']

@requires_dask
def test_get_dask_if_installed(self) -> None:
chunkmanager = guess_chunkmanager(None)
assert isinstance(chunkmanager, DaskManager)

@pytest.mark.skipif(has_dask, reason="requires dask not to be installed")
def test_dont_get_dask_if_not_installed(self) -> None:
with pytest.raises(ValueError, match="unrecognized chunk manager dask"):
def test_no_chunk_manager_available(self, monkeypatch) -> None:
monkeypatch.setattr("xarray.namedarray.parallelcompat.list_chunkmanagers", dict)
with pytest.raises(ImportError, match="no chunk managers available"):
guess_chunkmanager("dask")

@requires_dask
Expand Down
Loading