Fix CI breakage from matplotlib 3.11#672
Merged
Merged
Conversation
The wind rose regression test asserted byte-identical pixels against a committed baseline. This conflated code-induced changes (the refactoring safety it was meant to provide) with environment-induced rendering drift. matplotlib 3.11 produces ~22 RMS of sub-pixel drift, which broke the test across all CI jobs. Switch to matplotlib's compare_images with an RMS tolerance of 10 — well below a real layout/data change (~30 RMS) and above patch-level noise — and regenerate the baseline on matplotlib 3.11. compare_images also writes a diff image on failure for inspection.
matplotlib 3.11's stricter stubs broke the typecheck CI step on two pre-existing call sites: - _temporal_coverage.py: pass a tuple (not a list) to plt.xlim, matching the overload signature. - _wind_rose.py: the Legend 'loc' argument is a widened str from literal assignments; suppress with a targeted type: ignore, consistent with the surrounding legend block.
ecomodeller
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the project to keep CI green with the newly released matplotlib 3.11 by making the wind-rose regression test robust to small rendering drift and by addressing new mypy failures triggered by stricter matplotlib stubs.
Changes:
- Reworked the wind-rose regression test to use
matplotlib.testing.compare.compare_imageswith an RMS tolerance instead of byte-identical pixel comparison. - Adjusted plotting code to satisfy updated matplotlib type stubs (tuple for
plt.xlim, and a targetedtype: ignoreforLegend(..., loc=...)).
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/regression/test_regression_rose.py | Switches regression image assertion to compare_images with tolerance and updates baseline workflow comments. |
| src/modelskill/plotting/_wind_rose.py | Adds a mypy-targeted type: ignore[arg-type] for Legend(..., loc=...) under updated matplotlib stubs. |
| src/modelskill/plotting/_temporal_coverage.py | Uses a tuple for plt.xlim to match the matplotlib overloads. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+51
to
+55
| # Compare against the baseline within a tolerance. compare_images returns | ||
| # None on success and an explanatory message on failure, writing a | ||
| # *-failed-diff.png next to img_path for inspection. | ||
| result = compare_images(baseline_path, str(img_path), tol=IMAGE_TOLERANCE) | ||
| assert result is None, result |
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.
matplotlib 3.11 landed on PyPI and broke CI across every Linux job. Because
uv.lockis gitignored, CI always resolves the latest matplotlib, so this hitmaindirectly. Two distinct failures:1. Wind rose regression test (
teststep)The test (
tests/regression/test_regression_rose.py) asserted byte-identical pixels against a committed baseline, conflating code-induced changes (the refactoring safety it's meant to provide) with environment-induced rendering drift. matplotlib 3.11 renders the wind rose ~22 RMS off the old baseline.Switched to matplotlib's
compare_imageswith an RMS tolerance of 10 — comfortably below a real layout/data regression (~30 RMS, measured) and above patch-level noise — keeping the test a refactoring tripwire that's robust to minor drift. The baseline is regenerated on 3.11, andcompare_imageswrites a diff image on failure. matplotlib is intentionally not pinned (the unpinned matrix and the mikeio-main job track latest to catch upstream breakage early); the baseline is regenerated deliberately on a major bump.2. mypy errors from stricter 3.11 type stubs (
typecheckstep)_temporal_coverage.py: pass a tuple (not a list) toplt.xlim, matching the overload._wind_rose.py: theLegendlocargument is a widenedstrfrom literal assignments; targetedtype: ignore, consistent with the surrounding block.Out of scope (3.13 deprecation warnings, not current failures)
locsdeprecation (not our code).apply_theta_transformsin_taylor_diagram_external.py— deferred: removing it can alter Taylor diagram rendering and there's no image regression test to guard it; warns only, doesn't break on 3.11.