fix(palette): lazy matplotlib import — unbreaks ggplot2 + makie impl-generate#7713
Merged
Merged
Conversation
PR #7692 introduced a regression: moving `from .palette import …` to the top of core/images.py made `import core.images` transitively pull in matplotlib (via palette.py's module-level `from matplotlib.colors import LinearSegmentedColormap`). This broke `python -m core.images process` in the impl-generate workflow for non-Python libraries (ggplot2 / makie), where the runner only installs PIL helpers — no matplotlib. raincloud-basic R and Julia both auto-failed 3× on the post-render PNG optimisation step with ModuleNotFoundError: No module named 'matplotlib'. Fix: keep palette.py matplotlib-free at module level. The cmap factory (`diverging`) and the `imprint_seq` / `imprint_div_{light,dark}` attributes are now constructed lazily on first access via module `__getattr__`, so importing palette (and transitively, images) costs nothing for callers that only need the hex constants. Public API preserved: - `from core.palette import imprint_seq` still works - `core.palette.imprint_seq.name == "imprint_seq"` - `register_with_matplotlib()` still registers all three cmaps Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes the palette import layering regression by keeping core.palette free of module-level matplotlib imports, allowing PIL-only image processing paths for non-Python plot implementations to import core.images without installing matplotlib.
Changes:
- Moves matplotlib imports into colormap construction paths.
- Adds lazy module attribute resolution for
imprint_seqand diverging colormaps. - Updates matplotlib registration to resolve lazy colormaps before registering them.
|
|
||
| imprint_div_light: LinearSegmentedColormap = diverging("light") | ||
| imprint_div_dark: LinearSegmentedColormap = diverging("dark") | ||
| def __getattr__(name: str): |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Adds the LinearSegmentedColormap return annotation Copilot flagged on #7713. With `from __future__ import annotations` active, the annotation stays as a forward reference and does not trigger a runtime matplotlib import — `core.palette` (and transitively `core.images`) remains safe to import without matplotlib installed. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
import core.imagesstarted transitively requiring matplotlib viacore.palette's module-levelfrom matplotlib.colors import LinearSegmentedColormap.Process plot images (light + dark)step inimpl-generate.ymlfor non-Python libraries (ggplot2 / makie), where the runner only installs PIL helpers — not matplotlib. raincloud-basic R and Julia auto-failed 3× withModuleNotFoundError: No module named 'matplotlib'.core/palette.pymatplotlib-free at module level. Cmap factory (diverging) andimprint_seq/imprint_div_{light,dark}are constructed lazily on first attribute access via module__getattr__.Why this is the right layering
core/images.pyis documented as PIL-only — the whole point of the reviewer feedback on #7692 was that importing it must not pull matplotlib in as a side effect. R/Julia plot scripts hardcode hex codes; they don't need matplotlib for rendering. But the workflow callspython -m core.images process plot.pngafterwards to optimise the rendered PNG with PIL — and that's where the missing matplotlib bites.Public API preserved
from core.palette import imprint_seq✅ (lazy)core.palette.imprint_seq.name == "imprint_seq"✅register_with_matplotlib()still registers all three cmaps ✅Test plan
uv run ruff check .cleanuv run ruff format --check .cleanimport core.imagessucceeds,LIBRARY_COLORSpopulated🤖 Generated with Claude Code