Skip to content

Make matplotlib an optional [plot] extra (#2494)#2495

Merged
brendancol merged 3 commits into
mainfrom
issue-2494
May 27, 2026
Merged

Make matplotlib an optional [plot] extra (#2494)#2495
brendancol merged 3 commits into
mainfrom
issue-2494

Conversation

@brendancol

Copy link
Copy Markdown
Contributor

Closes #2494

matplotlib was listed in install_requires, but nothing in the package imports
it at module top level. Every use is a lazy import inside the .xrs.plot
accessor helpers. So a plain pip install xarray-spatial was dragging in
matplotlib plus its whole chain (pillow, fonttools, kiwisolver, contourpy,
cycler, pyparsing) for users who only run the compute functions.

  • Move matplotlib out of install_requires into a new plot extra.
  • Add a _require_matplotlib() guard at the two plot entry points in
    accessor.py. Without matplotlib, .xrs.plot() now raises a clear
    ImportError pointing at pip install xarray-spatial[plot] instead of a bare
    ModuleNotFoundError.
  • Add matplotlib to the tests extra so the existing plot tests keep running
    in CI.
  • Update README, install docs, and CHANGELOG.

pandas is intentionally left alone: xarray requires it and several core modules
import it at top level, so it can't move to an extra.

This is a compute-only change, not backend-specific. No spatial operation
imports matplotlib on any backend.

Test plan

  • pytest xrspatial/tests/test_accessor.py (29 passed) — includes new tests
    that block matplotlib in sys.modules and assert core modules still import
    and the plot helpers raise the friendly error.
  • pytest xrspatial/geotiff/tests/test_features.py -k "plot or palette or colormap" (8 passed)
  • Bare-import check: import xrspatial and the focal/zonal/dasymetric
    modules import with matplotlib blocked.

matplotlib was in install_requires but no compute path imports it; every
use is a lazy import inside the .xrs.plot accessor helpers. Move it to a
plot extra so a plain install skips matplotlib and its transitive chain
(pillow, fonttools, kiwisolver, contourpy, cycler, pyparsing).

Add a _require_matplotlib() guard at the two plot entry points so a
missing install raises a clear ImportError pointing at
pip install xarray-spatial[plot]. Add matplotlib to the tests extra so
the existing plot tests keep running in CI. Update README, install docs,
and CHANGELOG.
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label May 27, 2026

@brendancol brendancol left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

PR Review: Make matplotlib an optional [plot] extra (#2494)

Scope is packaging + two guard calls, so the geospatial-specific checks
(backends, dask, CUDA, numerics) don't apply here. I focused on whether the
dependency move is complete and the guard behaves.

Blockers

None.

Suggestions

  • _require_matplotlib() (accessor.py:24) checks import matplotlib but the
    call sites then do import matplotlib.pyplot as plt. matplotlib can import
    while pyplot fails (no usable backend, broken install). That's an uncommon
    case and the error would still be an ImportError, just not the friendly one.
    Checking import matplotlib.pyplot in the guard would close the gap. Not
    blocking.

Nits

  • test_compute_imports_without_matplotlib calls importlib.reload on
    xrspatial.focal, zonal, and dasymetric. The reload re-executes those
    modules in place, so other tests in the same session keep the reloaded
    objects. It passes today, but reloading numba/ngjit modules for an
    import-side-effect check is heavier than needed. Importing the modules in a
    subprocess with matplotlib blocked, or just asserting the import doesn't
    raise, would isolate the check better.

What looks good

  • The core finding holds: no compute path imports matplotlib. The only runtime
    uses are the two .xrs.plot accessor methods; the matches in zonal.py and
    bump.py are inside .. plot:: docstring examples, not executed code.
  • matplotlib added to the tests extra, so the existing plot/palette tests keep
    running in CI rather than silently skipping.
  • pandas correctly left as required, with the reason recorded.
  • _listed_colormap_from_attrs kept as-is; it already returns None on
    ImportError and sits on the read path, so guarding it would have broken
    non-plotting GeoTIFF reads.
  • New tests assert both the friendly-error path and that core modules import
    with matplotlib blocked.

Checklist

  • No compute path imports matplotlib (verified across the package)
  • Friendly ImportError points at the plot extra
  • tests extra keeps plot tests running in CI
  • Docs (README, installation.rst) and CHANGELOG updated
  • pandas correctly identified as non-removable
  • Guard covers matplotlib.pyplot, not just matplotlib (see suggestion)

- _require_matplotlib now imports matplotlib.pyplot so an installed-but-
  unusable matplotlib (no backend) also gets the friendly error.
- Replace the importlib.reload-based import test with a subprocess that
  imports the compute modules against a clean cache with matplotlib blocked.

@brendancol brendancol left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Follow-up review (after 1e1157d)

Both findings from the first pass are resolved.

  • Suggestion (guard pyplot): _require_matplotlib() now does
    import matplotlib.pyplot, so an installed-but-unusable matplotlib also
    produces the friendly error. Resolved.
  • Nit (reload-based test): test_compute_imports_without_matplotlib now runs
    in a subprocess against a clean module cache instead of reloading numba
    modules in-session. Resolved.

No new issues. pytest xrspatial/tests/test_accessor.py is 29 passed and
flake8 is clean on the changed files. No remaining items beyond CI.

@brendancol brendancol merged commit e89904b into main May 27, 2026
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance PR touches performance-sensitive code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move matplotlib to an optional [plot] extra to slim the default install

1 participant