refactor(plot)!: adopt cleopatra 0.31 RgbBands for the ArrayGlyph constructor - #972
Merged
Conversation
…structor cleopatra 0.31.0 (serapeum-org/cleopatra#291) grouped the four loose RGB band-prep constructor keywords (rgb / surface_reflectance / cutoff / percentile) into a single RgbBands object; ArrayGlyph.__init__ now takes only rgb_bands=. render_array builds an RgbBands from the resolved band indices and stretch controls on the RGB path (and passes rgb_bands=None on the single-band and animate paths, where prepare_array — which keeps its loose signature — already consumed the stretch). Bump the viz extra to cleopatra[tiles]>=0.31.0. BREAKING CHANGE: requires cleopatra >=0.31.0.
…int (L1) render_array requires cleopatra >= 0.31 for RgbBands, but require_cleopatra() only checks presence, so a stale cleopatra <0.31 raised a bare ImportError: cannot import name 'RgbBands'. Translate that ImportError into OptionalPackageDoesNotExist with the same [viz]-upgrade hint pyramids uses for other optional-extra version mismatches.
…or (M1, L2) No test referenced RgbBands. Add two mocked-ArrayGlyph routing tests: the RGB plot path builds an RgbBands carrying the band indices and all three stretch controls (surface_reflectance / cutoff / percentile — the L2 gap), and the single-band and animate paths pass rgb_bands=None. They construct a real RgbBands so they carry the module's plot marker (render_array needs the [viz] extra); CI runs the plot lane on every PR.
…rsion (L1, N1) Import ArrayGlyph/PanelLabels/PointOverlay unguarded and wrap only the RgbBands import, so a genuine cleopatra module-load failure keeps its real ImportError instead of being mis-branded as 'missing RgbBands' (L1). Drop the hardcoded cleopatra version from the error message — the floor lives solely in pyproject.toml's [viz] pin, which the upgrade hint points at (N1).
…are absent (N1, N2) Update the too-old-cleopatra test to match the version-free message, and assert the RGB routing test that the removed loose ctor keywords (rgb/surface_reflectance/ cutoff/percentile) do not leak onto the ArrayGlyph constructor.
|
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.



Description
Adopts cleopatra 0.31.0 (released on PyPI). 0.31 landed one consumer-facing breaking change to what pyramids
consumes — cleopatra serapeum-org/cleopatra#291 grouped
ArrayGlyph's four loose RGB band-prep constructorkeywords (
rgb/surface_reflectance/cutoff/percentile) into a singleRgbBandsobject, soArrayGlyph.__init__now takes onlyrgb_bands=. This PR wires pyramids' oneArrayGlyphconstruction site ontothe new object and bumps the pin.
The single call site.
render_array(src/pyramids/dataset/_plot_helpers.py) is the only place pyramidsconstructs a cleopatra
ArrayGlyph. It now builds anRgbBandsfrom the resolved band indices and stretchcontrols on the RGB path:
rgb_bands = RgbBands(rgb, surface_reflectance=…, cutoff=…, percentile=…).rgb_bands=None. The animate compositor still usesArrayGlyph.prepare_array(...), which keeps its loose keyword signature in 0.31, so the per-frame RGBcompositing is unchanged; the constructor's stretch params are nulled after compositing as before.
Every public facade (
Dataset.plot,DatasetCollection.plot,NetCDF.plot,Analysis.plot) forwards topyramids' own
render_array, whose signature is unchanged — so this is the only source change. pyramids' usersurface (
rgb_options=on the raster facades) is untouched;RgbBandsis an internal detail.Not affected. The rest of cleopatra#291 (
ColorBar.to_options/resolve,PointOverlay.draw,FrameLabel/PanelLabelsmethod moves, the removed baseGlyph._plot_point_values, the newDataStyle.for_apply_style) is internal cleopatra refactoring that pyramids never called — it drives the groupedrender objects through the public
plot/animate/facetAPI, whose shape is unchanged. cleopatra#298 wasdocs/housekeeping. No notebook or test needed changes: the
rgb_optionsfacade is pyramids' only RGB entry point,and nothing constructs a cleopatra
ArrayGlyphwith loosergb=outsiderender_array.Install source. cleopatra is the published
cleopatra[tiles]>=0.31.0via the[viz]extra;pixi.lockbumped0.30.0 → 0.31.0 across every environment.
Issues
ArrayGlyphkwargs, breaking all plotting.Type of change
How Has This Been Tested?
tests/dataset/plot tests/netcdf/plot tests/ugrid tests/basemap tests/feature/test_plot_cleopatra.py tests/dataset/collection/test_plot_labels.py→ 720 passed.RgbBandsconstruction): the-k "rgb or percentile or reflect"subset →39 passed — single-band, RGB percentile/reflectance stretch, and RGB-animate composites all render.
mypyclean on the changed module (cleopatra is typed via thefollow_untyped_importsoverride, so theRgbBandsconstruction is type-checked).ArrayGlyph.__init__exposesrgb_bands, no loosergb.Migration notes (breaking)
Only the pin changes for downstream users of the pyramids raster facades — the pyramids RGB surface is unchanged
(
ds.plot(rgb_options={"rgb": [...], "surface_reflectance": ..., "cutoff": ..., "percentile": ...})). The break ispurely the minimum cleopatra version: requires cleopatra >=0.31.0 (0.30.x no longer satisfies the pin, and
0.30's loose
ArrayGlyph(rgb=...)constructor is gone upstream).Behaviour change carried in by the bump (not a pyramids code change). cleopatra 0.31 also ships a semantic
fix in serapeum-org/cleopatra#291: the RGB surface-reflectance
cutoffnow clips each band's data rather than theinteger band index. Callers that pass
cutoffinrgb_options(still-RGB and RGB-animate paths alike) will renderslightly different pixels than on cleopatra 0.30 — this is an upstream correctness improvement, surfaced here so it
is discoverable. No other pyramids-visible output changes with the bump.
Checklist:
new path; 720 plot tests green)