Summary
chemotools.utils.discovery.all_estimators() (and all_displays(), all_functions()) raise an uncaught ImportError if matplotlib is not installed, even though the caller may only be interested in non-plotting estimators (e.g. transformers in chemotools.augmentation, chemotools.baseline, etc.). matplotlib is declared as an optional dependency (the viz extra in pyproject.toml), but discovery doesn't treat it as optional.
Steps to reproduce
# with chemotools installed WITHOUT the `viz` extra (no matplotlib)
from chemotools.utils.discovery import all_estimators
all_estimators()
Actual behavior
ImportError: 'chemotools.inspector' requires the optional dependency 'matplotlib'. Install it with: pip install chemotools[viz]
Confirmed on chemotools 0.4.0, reproduced by blocking matplotlib imports in a subprocess and calling all_estimators().
Expected behavior
all_estimators() should return the list of estimators that can be imported (everything outside chemotools.plotting and chemotools.inspector). Discovery shouldn't hard-fail because of an optional, unrelated dependency.
Root cause
chemotools/utils/discovery.py walks every submodule with pkgutil.walk_packages and calls import_module(module_name) on each one with no error handling:
chemotools.plotting and chemotools.inspector guard their init.py with a package-import-time check:
import_optional_dependency(
"matplotlib",
caller_name="chemotools.plotting",
extra_name="viz",
)pkgutil.walk_packages itself tolerates this fine internally (it uses onerror=None when recursing into packages, so it silently gives up trying to descend into chemotools.plotting / chemotools.inspector). But it still yields "chemotools.plotting" and "chemotools.inspector" as module names from the parent package's listing. The discovery loop then calls import_module() on those names itself, hits the same ImportError again, and this time nothing catches it, so the whole all_estimators() call fails.
Comparison with scikit-learn
chemotools' discovery code is adapted directly from sklearn's:
Note that sklearn's all_estimators / all_displays / all_functions also call import_module() with no try/except around it, so at first glance the same "bug" exists there. It never triggers, though, because sklearn never makes a package import conditional on matplotlib.
sklearn's optional-matplotlib helper is:
The crucial difference is where it's called: lazily, inside .plot(), .from_estimator(), or .from_predictions() of *Display classes, never at module or package import time. Axes / Figure type hints in those classes are deferred (from future import annotations plus TYPE_CHECKING), so sklearn.metrics and friends import cleanly with zero matplotlib installed. RocCurveDisplay etc. are always discoverable via all_displays(); only calling .plot() on them requires matplotlib and raises the helpful error.
scikit-image went through the same exercise when making matplotlib optional:
In chemotools, by contrast, matplotlib is imported at module scope throughout chemotools/plotting/* and chemotools/inspector/* (import matplotlib.pyplot as plt, from matplotlib.axes import Axes, from matplotlib.figure import Figure, even a colormap registration side effect in chemotools/plotting/_utils.py), and the package init.py files turn that into a hard import-time failure for the whole subpackage. That's what breaks the sklearn-derived discovery pattern.
CI gap
This isn't caught by CI because:
- .github/workflows/ci.yml only runs the tests-X.Y and tests-min-sklearn-X.Y nox sessions, both of which install with extras=("viz",), so matplotlib is always present.
- noxfile.py has a tests-core session intended for the no-viz case, but it's not part of the CI matrix at all.
- Even if it were run, tests-core ignores tests/plotting and tests/inspector but not tests/utils/test_discovery.py, which would itself fail on all_estimators() / all_displays() / all_functions() for the same reason.
Possible fixes
- Minimal: in chemotools/utils/discovery.py, wrap each import_module(module_name) in try/except ImportError: continue. This mirrors the tolerance pkgutil.walk_packages(onerror=None) already applies internally, and immediately fixes the crash for chemotools.plotting / chemotools.inspector (and any future optional-dependency-gated subpackage).
- sklearn-parity (larger): make chemotools.plotting / chemotools.inspector always importable without matplotlib by dropping the package-level import_optional_dependency("matplotlib", ...) guards, deferring Axes / Figure typing via TYPE_CHECKING, and moving matplotlib usage into the .show() / .render() methods (raising via import_optional_dependency only when a user actually requests a plot). This gives full parity with sklearn's Display pattern, so all_displays() would list SpectraPlot, ScoresPlot, etc. even without matplotlib, but it touches roughly 20 files.
Summary
chemotools.utils.discovery.all_estimators()(andall_displays(),all_functions()) raise an uncaughtImportErrorif matplotlib is not installed, even though the caller may only be interested in non-plotting estimators (e.g. transformers inchemotools.augmentation,chemotools.baseline, etc.). matplotlib is declared as an optional dependency (thevizextra inpyproject.toml), but discovery doesn't treat it as optional.Steps to reproduce
Actual behavior
ImportError: 'chemotools.inspector' requires the optional dependency 'matplotlib'. Install it with: pip install chemotools[viz]
Confirmed on chemotools 0.4.0, reproduced by blocking matplotlib imports in a subprocess and calling all_estimators().
Expected behavior
all_estimators() should return the list of estimators that can be imported (everything outside chemotools.plotting and chemotools.inspector). Discovery shouldn't hard-fail because of an optional, unrelated dependency.
Root cause
chemotools/utils/discovery.py walks every submodule with pkgutil.walk_packages and calls import_module(module_name) on each one with no error handling:
chemotools.plotting and chemotools.inspector guard their init.py with a package-import-time check:
pkgutil.walk_packages itself tolerates this fine internally (it uses onerror=None when recursing into packages, so it silently gives up trying to descend into chemotools.plotting / chemotools.inspector). But it still yields "chemotools.plotting" and "chemotools.inspector" as module names from the parent package's listing. The discovery loop then calls import_module() on those names itself, hits the same ImportError again, and this time nothing catches it, so the whole all_estimators() call fails.
Comparison with scikit-learn
chemotools' discovery code is adapted directly from sklearn's:
Note that sklearn's all_estimators / all_displays / all_functions also call import_module() with no try/except around it, so at first glance the same "bug" exists there. It never triggers, though, because sklearn never makes a package import conditional on matplotlib.
sklearn's optional-matplotlib helper is:
The crucial difference is where it's called: lazily, inside .plot(), .from_estimator(), or .from_predictions() of *Display classes, never at module or package import time. Axes / Figure type hints in those classes are deferred (from future import annotations plus TYPE_CHECKING), so sklearn.metrics and friends import cleanly with zero matplotlib installed. RocCurveDisplay etc. are always discoverable via all_displays(); only calling .plot() on them requires matplotlib and raises the helpful error.
scikit-image went through the same exercise when making matplotlib optional:
In chemotools, by contrast, matplotlib is imported at module scope throughout chemotools/plotting/* and chemotools/inspector/* (import matplotlib.pyplot as plt, from matplotlib.axes import Axes, from matplotlib.figure import Figure, even a colormap registration side effect in chemotools/plotting/_utils.py), and the package init.py files turn that into a hard import-time failure for the whole subpackage. That's what breaks the sklearn-derived discovery pattern.
CI gap
This isn't caught by CI because:
Possible fixes