diff --git a/dascore/exceptions.py b/dascore/exceptions.py index 2c55abca..d6146740 100644 --- a/dascore/exceptions.py +++ b/dascore/exceptions.py @@ -118,7 +118,20 @@ class InvalidIndexVersionError(InvalidIndexError): class MissingOptionalDependencyError(ImportError, DependencyError): - """Raised when an optional package needed for some functionality is missing.""" + """ + Raised when an optional package needed for some functionality is missing. + + The install_name attribute, when set, gives the name of the package to + install (eg protobuf) which may differ from the import name + (eg google.protobuf). It defaults on the class so subclasses which don't + call this init still have it. + """ + + install_name: str | None = None + + def __init__(self, *args, install_name: str | None = None, **kwargs): + super().__init__(*args, **kwargs) + self.install_name = install_name class DASVaderCompatibilityError(InvalidFiberFileError, DependencyError): diff --git a/dascore/io/core.py b/dascore/io/core.py index 4f838424..b9eb9615 100644 --- a/dascore/io/core.py +++ b/dascore/io/core.py @@ -6,6 +6,7 @@ from __future__ import annotations import inspect +import re import warnings from collections import defaultdict from collections.abc import ( @@ -61,6 +62,8 @@ from dascore.utils.io import IOResourceManager, get_handle_from_resource from dascore.utils.mapping import FrozenDict from dascore.utils.misc import ( + _get_install_message, + _get_install_name, _iter_filesystem, _locked, _reinit_after_fork, @@ -1298,6 +1301,23 @@ def _count_generator(generator): return entity_count +_MISSING_MODULE_PATTERN = re.compile(r"^(\S+) is not installed") + + +def _get_missing_install_name(exception: MissingOptionalDependencyError) -> str: + """Get the installable package name from a missing dependency error.""" + if exception.install_name: + return exception.install_name + # Errors raised outside of optional_import (eg by a third party FiberIO) + # identify the module, if at all, with the module name or the message form + # optional_import used to use. Any other message could say anything, so + # nothing is recommended for installation. + if not (name := exception.name or ""): + match = _MISSING_MODULE_PATTERN.match(exception.msg or "") + name = match.group(1) if match else "" + return _get_install_name(name) + + def _handle_missing_optionals(output_count, optional_dep_dict): """ Inform the user there are files that can be read but the proper @@ -1306,10 +1326,17 @@ def _handle_missing_optionals(output_count, optional_dep_dict): If there are other readable files that were found, raise a warning. Otherwise, raise a MissingOptionalDependencyError. """ + counts = ", ".join( + f"{name or 'unknown'} ({count} files)" + for name, count in sorted(optional_dep_dict.items()) + ) + # Unidentifiable packages can't be included in an install command. + packages = [x for x in optional_dep_dict if x] + install = f" {_get_install_message(packages)}" if packages else "" msg = ( f"DASCore found files that can be read if additional packages are " f"installed. The needed packages and the found number of files are: " - f"{dict(optional_dep_dict)}" + f"{counts}.{install}" ) warn_or_raise( msg, @@ -1416,7 +1443,7 @@ def _iter_scan_results( scan_kwargs["snap"] = snap source = fiber_io.scan(resource, **scan_kwargs) except MissingOptionalDependencyError as ex: - missing_optional_deps[ex.msg.split(" ")[0]] += 1 + missing_optional_deps[_get_missing_install_name(ex)] += 1 continue # scan() is best-effort across many resources, so surface # dependency/compatibility problems as warnings and keep diff --git a/dascore/utils/jit.py b/dascore/utils/jit.py index 249c4cc0..2f91a65b 100644 --- a/dascore/utils/jit.py +++ b/dascore/utils/jit.py @@ -7,6 +7,8 @@ import warnings from functools import wraps +from dascore.utils.misc import _get_install_message + class _DummyNumba: """A simple class for acting like numba when numba is not installed.""" @@ -89,6 +91,7 @@ def decorated(*args, **kwargs): msg = ( f"{func.__name__} requires python module " f"numba but it is not installed. " + f"{_get_install_message('numba')}" ) raise ImportError(msg) else: diff --git a/dascore/utils/misc.py b/dascore/utils/misc.py index 47363553..ece24fcc 100644 --- a/dascore/utils/misc.py +++ b/dascore/utils/misc.py @@ -441,6 +441,40 @@ def iterate(obj): return obj if isinstance(obj, Iterable) else (obj,) +# Import names whose installable package name differs from the import name. +_INSTALL_NAMES = { + "google.protobuf": "protobuf", + "yaml": "pyyaml", +} + + +def _get_install_name(import_name: str) -> str: + """Get the package to install which provides the module import_name.""" + parts = import_name.split(".") + # Search most to least specific so sub-modules resolve to their parent. + for stop in range(len(parts), 0, -1): + if (name := _INSTALL_NAMES.get(".".join(parts[:stop]))) is not None: + return name + return parts[0] + + +def _get_install_message(packages: str | Iterable[str]) -> str: + """Get a message telling the user how to install one or more packages.""" + names = " ".join(sorted(iterate(packages))) + return f"Install with `pip install {names}` or `uv pip install {names}`." + + +def _is_missing_module(import_name: str, error: ImportError) -> bool: + """Determine if an ImportError means import_name itself is not installed.""" + # Only the import machinery proves a module is absent, and it names the + # module it failed to find. Any other error (eg an installed package + # raising from its __init__) means the failure happened inside code which + # is installed, as does one naming a different module. + if not isinstance(error, ModuleNotFoundError) or not (failed := error.name or ""): + return False + return import_name == failed or import_name.startswith(f"{failed}.") + + @overload def optional_import( package_name: str, @@ -498,8 +532,25 @@ def optional_import( """ try: mod = importlib.import_module(package_name) - except ImportError: - msg = f"{package_name} is not installed but is required for {required_for}" + except ImportError as ex: + install_name = _get_install_name(package_name) + if _is_missing_module(package_name, ex): + msg = ( + f"{package_name} is not installed but is required for " + f"{required_for}. {_get_install_message(install_name)}" + ) + else: + # The package is installed; something it imports is not, so + # installing it again wouldn't help. + install_name = None + msg = ( + f"{package_name} could not be imported ({ex}) but is " + f"required for {required_for}." + ) + # Raise here (rather than with warn_or_raise) so the install name is + # attached to the exception for callers which aggregate them. + if on_missing == "raise": + raise MissingOptionalDependencyError(msg, install_name=install_name) from ex warn_or_raise(msg, MissingOptionalDependencyError, behavior=on_missing) mod = None return mod diff --git a/docs/changelog.qmd b/docs/changelog.qmd index 1893351d..8cdde9dd 100644 --- a/docs/changelog.qmd +++ b/docs/changelog.qmd @@ -4,6 +4,7 @@ The [releases page](https://github.com/DASDAE/dascore/releases) tracks changes f ## Unreleased API Changes +- Missing optional dependency messages now name the package to install and give the command to install it. The import name and the package name are not always the same, so `dc.scan` reporting `{'google.protobuf.descriptor_pb2': 10308}` left users guessing; it now reports `protobuf (10308 files)` along with ``Install with `pip install protobuf` or `uv pip install protobuf` ``. `MissingOptionalDependencyError` also carries an `install_name` attribute, and an `ImportError` raised inside an installed package is reported as a failed import rather than a missing install. - The fiber IO format readers now share two helpers instead of each carrying its own copy of the same scaffolding: [`make_scan_payload`](`dascore.io.make_scan_payload`) builds one `FiberIO.scan` payload (taking `dims` and `shape` from the coords unless given), and `dascore.io.utils.build_patches` performs the common `read` tail of trim, drop-if-empty, attach attrs. Two side effects for readers: an already empty source now yields no patch from the `APSensing` and `HDAS` readers rather than a zero-size one (matching every other format), and the `GDR_DAS` and `Neubrex` readers declare `time`/`distance` explicitly rather than absorbing them from `**kwargs`. The unused, never-populated `ProdMLPatchAttrs` classes are removed from `dascore.io.prodml.core` and `dascore.io.dashdf5.core`; `ProdMLRawPatchAttrs` in `dascore.io.prodml.utils` is the one the reader uses. Relatedly, evenly sampled coordinates are now built from a sample count rather than a hand-computed stop, so a source declaring zero samples yields an empty coordinate instead of raising a validation error. - Fixed chunking a coordinate whose units are not the canonical SI unit (e.g. a distance in feet). Plan trims carry canonical SI magnitudes, which were applied to the patch coordinate as bare numbers, so each piece covered the wrong physical interval and samples were silently dropped — chunking a 300 channel patch in feet returned 61 channels. The trim now converts to the coordinate's own units at load, the same conversion `Spool.select` already performed. - **`Spool.chunk` accepts quantities as the chunk length.** A quantity in the coordinate's own units works (`chunk(time=10 * dc.units.s)`, `chunk(distance=100 * dc.units.ft)`), where any quantity previously raised `NotImplementedError`. A quantity of information (`chunk(time=25 * dc.units.megabytes)`) chunks so each patch's *data array* is at most the requested size; the sample count is floored, and a partition mixing element types is sized against the dtype assembly upcasts to. `overlap` accepts both forms, and `Spool.chunk_plan(...).params["size"]` reports what a size resolved to. The index schema records each patch's element dtype to make this possible, so its version is bumped and existing indexes must be deleted and rebuilt. An attr named `dtype` is now reserved and stays unindexed. diff --git a/tests/test_io/test_io_core.py b/tests/test_io/test_io_core.py index 2d4f5ba1..dbc98729 100644 --- a/tests/test_io/test_io_core.py +++ b/tests/test_io/test_io_core.py @@ -31,7 +31,9 @@ from dascore.io.core import ( FiberIO, _FiberIOManager, + _get_missing_install_name, _get_reloadable_source_path, + _handle_missing_optionals, _reinit_manager_lock, _resolve_read_spool, _scan_result_to_summary, @@ -1106,8 +1108,10 @@ def test_scan_missing_optional_dependency_raises(self, tmp_path): path.write_text("placeholder") msg = "found files that can be read if additional packages" - with pytest.raises(MissingOptionalDependencyError, match=msg): + with pytest.raises(MissingOptionalDependencyError, match=msg) as exc_info: dc.scan(path) + # The message should say how to install the missing package. + assert "pip install not_optional_pkg" in str(exc_info.value) def test_scan_missing_optional_dependency_warns_with_other_outputs(self, tmp_path): """Scan should warn if optional deps are missing but other files load.""" @@ -1611,6 +1615,39 @@ def test_get_supported_io_table(self): assert len(result_df) > 0 +class TestMissingInstallName: + """Tests for guessing the package to install from a dependency error.""" + + def test_name_sources(self): + """The name comes from the attr, the module, or the legacy message.""" + errors = [ + MissingOptionalDependencyError("blah", install_name="protobuf"), + MissingOptionalDependencyError("blah", name="google.protobuf"), + MissingOptionalDependencyError("protobuf is not installed but..."), + ] + assert [_get_missing_install_name(x) for x in errors] == ["protobuf"] * 3 + + def test_unidentifiable_package(self): + """Arbitrary messages should not be mistaken for package names.""" + error = MissingOptionalDependencyError("Optional dependency foo is missing") + assert _get_missing_install_name(error) == "" + assert _get_missing_install_name(MissingOptionalDependencyError()) == "" + # Subclasses which skip the init still have an install name. + assert MissingOptionalDependencyError.install_name is None + + def test_message_omits_unknown_packages(self): + """Only identified packages belong in the install command.""" + with pytest.raises(MissingOptionalDependencyError) as exc_info: + _handle_missing_optionals(0, {"": 2, "segyio": 1, "protobuf": 3}) + msg = str(exc_info.value) + assert "unknown (2 files)" in msg + assert "pip install protobuf segyio" in msg + # Nothing should be recommended when no package was identified. + with pytest.raises(MissingOptionalDependencyError) as exc_info: + _handle_missing_optionals(0, {"": 2}) + assert "pip install" not in str(exc_info.value) + + class TestIOCoreCoverageEdges: """Remaining io.core resolution/robustness branches.""" diff --git a/tests/test_utils/test_misc.py b/tests/test_utils/test_misc.py index 392b596d..7b6fbc4b 100644 --- a/tests/test_utils/test_misc.py +++ b/tests/test_utils/test_misc.py @@ -18,6 +18,7 @@ import dascore as dc from dascore.exceptions import MissingOptionalDependencyError from dascore.utils.misc import ( + _get_install_name, _iter_filesystem, _locked, _spool_map, @@ -419,6 +420,15 @@ def test_all_null_false(self, diffs): assert not all_diffs_close_enough(diffs) +def _raise_error(error): + """Return a function which raises error when called.""" + + def _func(*args, **kwargs): + raise error + + return _func + + class TestOptionalImport: """Ensure the optional import works.""" @@ -439,6 +449,56 @@ def test_ignore(self): out = optional_import("boblib4", on_missing="ignore") assert out is None + def test_message_has_install_instructions(self, monkeypatch): + """The error should say how to install the package, not the module.""" + error = ModuleNotFoundError("No module named 'google'", name="google") + monkeypatch.setattr( + "importlib.import_module", _raise_error(error), raising=True + ) + with pytest.raises(MissingOptionalDependencyError) as exc_info: + optional_import("google.protobuf.descriptor_pb2") + msg = str(exc_info.value) + assert "pip install protobuf" in msg + assert "uv pip install protobuf" in msg + assert exc_info.value.install_name == "protobuf" + assert exc_info.value.__cause__ is error + + @pytest.mark.parametrize( + "error", + [ + # An installed package which imports something missing. + ModuleNotFoundError("No module named 'bob'", name="bob"), + # An installed package raising from its own __init__, either + # unnamed or naming itself. + ImportError("dascore.core requires the C extension"), + ImportError("dascore.core is broken", name="dascore.core"), + ], + ) + def test_broken_install_gives_no_install_advice(self, monkeypatch, error): + """An import failing inside an installed package isn't fixed by install.""" + monkeypatch.setattr( + "importlib.import_module", _raise_error(error), raising=True + ) + with pytest.raises(MissingOptionalDependencyError) as exc_info: + optional_import("dascore.core") + msg = str(exc_info.value) + assert "could not be imported" in msg + assert str(error) in msg + assert "pip install" not in msg + assert exc_info.value.install_name is None + assert exc_info.value.__cause__ is error + + +class TestGetInstallName: + """Tests for mapping import names to installable package names.""" + + def test_install_names(self): + """Sub-modules resolve to the package which provides them.""" + assert _get_install_name("xarray") == "xarray" + assert _get_install_name("dascore.utils.misc") == "dascore" + assert _get_install_name("google.protobuf.message") == "protobuf" + assert _get_install_name("yaml") == "pyyaml" + class TestGetStencilCoefficients: """Tests for stencil coefficients."""