refactor(plot)!: migrate to cleopatra 0.30 and remove the loose styling kwargs - #946
Open
MAfarrag wants to merge 27 commits into
Open
refactor(plot)!: migrate to cleopatra 0.30 and remove the loose styling kwargs#946MAfarrag wants to merge 27 commits into
MAfarrag wants to merge 27 commits into
Conversation
cleopatra 0.29 restructures its flat modules into subpackages with no re-export shim, so the imports move. Track git main (0.29.0) via a pixi pypi-dependency ahead of the PyPI/conda-forge release; the published [viz] extra stays a version spec so built wheels carry no direct-URL metadata. Bump the [viz] floor to >=0.29.0 and drop the git source once cleopatra 0.29 is released.
cleopatra 0.29 moved its flat modules into subpackages (glyphs/{gridded,primitives,
stats}, styling/, basemap/) and re-exports nothing from the root, so every import
path changes:
- array_glyph -> glyphs.gridded.array_glyph (ArrayGlyph/FrameLabel/PointOverlay);
ColorBar -> styling.colorbar; mesh_glyph/vector_glyph -> glyphs.gridded.*;
scatter_glyph/polygon_glyph -> glyphs.primitives.*; colors/styles -> styling.*;
geo/reference/tiles -> basemap.*; statistical_glyph.StatisticalGlyph ->
glyphs.stats.histogram_glyph.HistogramGlyph (renamed). Config path is unchanged.
- Update the pyramids.plot lazy shim and every test importorskip/@patch target and
the cleopatra >=0.28 gates to >=0.29.
Adopt the two behaviours the restructure unblocked:
- ArrayGlyph.facet now accepts colorbar=ColorBar (cleopatra#271), so fold the loose
cbar_* into a ColorBar on the facet path like plot/animate and drop the
RENDER_ONLY_OVERRIDES workaround.
- MeshGlyph.plot now accepts colorbar=ColorBar, so plot_mesh_data takes a typed
ColorBar (was bool-only).
Refs #933, #934.
… git source PyPI cleopatra 0.29.0 is the pre-restructure flat layout; the subpackage restructure lands in 0.30.0. Keep installing from git main (self-reports 0.29.0, carries the new layout) and bump the [viz] floor to >=0.30.0 (dropping the git source) at that release.
Mirror cleopatra 0.29's typed plot surface on the pyramids facades so the render controls are discoverable on the signature instead of buried in an untyped `**kwargs: Any`: - Dataset.plot: hoist `colorbar` / `points` / `kind` / `title` to explicit params and type the tail as `**kwargs: Unpack[PlotKwargs]`. - DatasetCollection.plot: hoist `colorbar` / `points` and the pyramids-only `animation_axis_values` override to explicit params; type the tail as `**kwargs: Unpack[AnimateKwargs]` (`title` already lives in AnimateKwargs). - NetCDF.plot: hoist `colorbar` / `points` for parity. The method is a multi-mode facade (static / animate / facet) whose valid kwargs differ per mode, so its tail stays `**kwargs: Any`; the hoisted params are only forwarded when set so a default never reaches the facet path (which rejects `points`). cleopatra ships inline types but no `py.typed`, so add a mypy override that follows its source types (else `Unpack[PlotKwargs]` collapses to `Unpack[Any]`), plus a narrow `arg-type` suppression on the single dynamic-dispatch backend whose runtime-built `**dict` cannot be statically matched to the now-typed cleopatra signatures.
…ot params Follow-up to the .plot/.animate kwargs alignment: - Add tests/netcdf/plot/test_hoisted_render_params.py covering the genuinely new NetCDF surface: the hoisted `colorbar` / `points` render on the static path, the conditional-injection contract (a default `None` is never forwarded), and the facet edge — a bare facet renders while an explicit `points` on the facet path raises a clear error. (Dataset `points` and Collection `animation_axis_values` were already covered.) - Document the hoisted params as explicit `Args` on all three facades: `colorbar` / `points` / `kind` / `title` on Dataset.plot; `colorbar` / `points` / `animation_axis_values` moved out of the DatasetCollection.plot `**kwargs` table into Args (beside the already-explicit `frame_label`); `colorbar` / `points` on NetCDF.plot with the facet caveat noted.
cleopatra's next restructure (grouped plot/animate/facet parameters) removed the flat styling keywords and the shims that accepted them, so a loose `color_scale=` / `style=` / `levels=` / bare-array `points=` now raises. Keep pyramids' stable loose-kwarg facade and translate to the typed group objects at the single render boundary: - add `_build_grouped_render_specs` folding loose color_scale/gamma/line_threshold/ line_scale/bounds/midpoint -> ColorScaling, levels/label_kw -> Contour, display_cell_value/num_size/background_color_threshold -> CellValues, and style/hillshade -> DataStyle (preserving the style=None / falsy-hillshade no-op). - render_array: build the groups, wrap a bare points array in PointOverlay, forward the groups on the plot/animate/facet render call, and translate facet col_coords/row_coords -> PanelLabels and figsize -> figure_size. - plot_mesh_data: same translation for MeshGlyph.plot (no cell-value overlay). - drop the obsolete style/hillshade version gate (the grouped API requires cleopatra >= 0.30, so the >= 0.24 upgrade hint no longer applies); update the tests that asserted the old loose-kwarg forwarding / version gate. - pin the cleopatra git source to the exact rev carrying the restructure so pixi re-fetches and the build is reproducible. Full plot surface (dataset/netcdf/ugrid/basemap/feature/collection): 1669 passed; mypy clean; doctests pass.
…th the raster family Give the mesh and vector plot facades the same explicit render-control surface as Dataset.plot / DatasetCollection.plot (colorbar / points / kind / title), forwarding what each backend supports and accepting the rest as documented no-ops: - UgridDataset.plot: hoist colorbar (forwarded to MeshGlyph.plot only when set, so the drawn-bar default survives) plus points / kind. A mesh has no point overlay and a fixed tripcolor/tricontour renderer, so points / kind are accepted for signature symmetry and ignored. - FeatureCollection.plot: hoist colorbar / points / kind / title across both engines. On engine="geopandas", colorbar toggles the geopandas legend and title is set on the returned Axes; on engine="cleopatra", colorbar / title now forward to the ScatterGlyph / PolygonGlyph plot() call (they were previously dropped by filter_kwargs, which only keeps constructor options). points / kind are vector no-ops. Tests: colorbar reaches / omitted-preserves-default / explicit-False on the mesh path; colorbar+title reach the glyph and points/kind are dropped on the cleopatra engine; title sets the Axes title and colorbar toggles the legend on the geopandas engine. feature + ugrid suites: 949 passed; mypy clean; doctests pass.
Mirror the colorbar / points wiring for the remaining cleopatra render-parameter dataclasses so every pyramids .plot method takes them as explicit, typed params (discoverable on the signature and accepted by mypy — they are not in the PlotKwargs/AnimateKwargs tails): - re-export ColorScaling / Contour / CellValues / DataStyle / Classify / PanelLabels through pyramids.plot (alongside ColorBar / PointOverlay / FrameLabel / Basemap). - Dataset.plot / DatasetCollection.plot / NetCDF.plot: color / contour / cells / data_style. UgridDataset.plot: color / contour / data_style (MeshGlyph has no cell-value overlay). FeatureCollection.plot: color / contour / classify, forwarded to the ScatterGlyph / PolygonGlyph plot() on engine="cleopatra" and ignored on engine="geopandas". - an explicitly-passed group wins over the one built from the loose kwargs: render_array and plot_mesh_data merge the loose-translated groups with setdefault, and the facades fold in only the set groups (nonnull_group_kwargs) so an unset group never blocks the translation or overrides a backend default. Tests: render_array precedence (explicit color wins over color_scale) + loose-still-builds + contour/cells/data_style reach; per-facade reach tests (Dataset via render_array, NetCDF, DatasetCollection, UgridDataset, FeatureCollection) and the geopandas-ignores-groups case; pyramids.plot re-export resolution for the new specs. Full plot surface: 1694 passed; mypy clean; doctests pass.
…kwargs cleopatra 0.30.0 is released; switch pyproject off the git pin to the published `cleopatra[tiles]>=0.30.0`. With the grouped render parameters now the upstream API, drop pyramids' backward-compat translation shims so the loose styling kwargs are no longer accepted — the typed group objects are the only way. - remove _build_grouped_render_specs and _migrate_deprecated_plot_specs. The loose forms cleopatra removed (color_scale / gamma / bounds / midpoint / line_* / levels / label_kw / display_cell_value / num_size / background_color_threshold / style / hillshade / point_* / pid_*) now raise cleopatra's "moved onto a grouped parameter object" error. - reject the loose cbar_* / ticks_spacing colour-bar kwargs in pyramids (cleopatra still tolerates them, but ColorBar is the single typed colour-bar surface) with a pointer to colorbar=ColorBar(...). cbar_kwargs (raw matplotlib dict) and add_colorbar stay valid. - rewire the NetCDF ColorOpts bag to emit contour=Contour(levels=) and data_style=DataStyle(style=, hillshade=) instead of loose kwargs. - migrate the paletted-raster path (#913) to color=ColorScaling.boundary(bounds=). - refresh every plot docstring that referenced the removed loose forms. BREAKING CHANGE: the loose plot styling keywords (color_scale, gamma, bounds, midpoint, line_threshold, line_scale, levels, label_kw, display_cell_value, num_size, background_color_threshold, style, hillshade, point_color/point_size/point_label_*/pid_*, cbar_label/cbar_length/cbar_orientation/cbar_label_*/cbar_location/cbar_inside/cbar_box/ cbar_tick_color/ticks_spacing) are removed; pass the typed group objects instead (color=ColorScaling, contour=Contour, cells=CellValues, data_style=DataStyle, points=PointOverlay, colorbar=ColorBar). Full plot surface: 1676 passed; mypy clean; doctests pass.
MAfarrag
marked this pull request as ready for review
August 11, 2026 19:03
MAfarrag
changed the title
…lot facades
Lever 1 signature simplification: the four Sentinel-imagery kwargs (rgb /
surface_reflectance / cutoff / percentile) were already deprecated on Dataset.plot and
DatasetCollection.plot in favour of the grouped rgb_options= dict — remove the loose
forms so the facades take rgb_options only.
- Dataset.plot: 19 -> 15 params; DatasetCollection.plot: 17 -> 13.
- replace Dataset._merge_rgb_options (loose+group merge with DeprecationWarnings) with a
lean Dataset._unpack_rgb_options(rgb_options) — dict-only, same unknown-key ValueError.
- the internal engines (Analysis.plot, render_array) keep rgb / surface_reflectance /
cutoff / percentile; only the public facades lose the loose forms.
- refreshed docstrings and deleted the obsolete loose-kwarg deprecation / collision tests.
BREAKING CHANGE: Dataset.plot / DatasetCollection.plot no longer accept the top-level
rgb / surface_reflectance / cutoff / percentile kwargs; pass them via
rgb_options={"rgb": ..., "surface_reflectance": ..., "cutoff": ..., "percentile": ...}.
Full plot surface: 1665 passed; mypy clean; doctests pass.
Align the abstract plot contract with the concrete facades: drop the loose rgb / surface_reflectance / cutoff params (removed in the previous commit) and declare the grouped rgb_options= dict instead. RasterBase.plot is an @AbstractMethod (body pass), so this is a signature + docstring change only. Updated the ABC signature-contract tests: replace the rgb / surface_reflectance default-checks with an rgb_options default-check and a guard that the loose Sentinel params are gone. Full plot surface: 1665 passed; mypy clean.
… plot facades Review M1: the loose styling kwargs were removed and now raise, but the colorbar param docstrings on Dataset.plot / DatasetCollection.plot and the RasterBase.plot ABC still described cbar_* / ticks_spacing as 'deprecated (still accepted, DeprecationWarning)', and the ABC still documented color_scale / gamma / bounds / display_cell_value / ... as valid **kwargs. Reword them to state the loose forms were removed and now raise ValueError, and redirect the ABC block to the typed color / contour / cells / data_style groups.
…e validation Review L1: the render_array docstring still advertised a ValueError 'if color_scale is not a recognised ColorScale value', but that pyramids-side validation was deleted in this branch. Reword to cover the cbar_* rejection and note the other removed loose kwargs raise cleopatra's own error.
…warg translation Review L2: the render-group hoist comments across the four plot facades said unset groups are dropped so they don't 'block render_array's loose-kwarg translation'. That translation (_migrate_deprecated_plot_specs / _build_grouped_render_specs) was removed in this branch; reword to the real reason — an unset group must not override cleopatra's backend default.
…-precedence tests Review L3: the two 'explicit color wins over loose color_scale' tests passed a dead color_scale= kwarg alongside the group and asserted only that the group is forwarded — a vacuous precedence claim now that the loose->group translation is gone (against real cleopatra the loose color_scale would itself raise). Reword the render_array test to a plain color-forwarding assertion (no color_scale=) and delete the mesh one (redundant with test_color_and_data_style_reach_mesh_glyph_plot).
Review L4 + N1: two color_scale reject-test docstrings described removed behavior (a pyramids-side value-level validation / cleopatra 'validating against the ColorScale enum') — cleopatra 0.30 rejects the color_scale *key* regardless of value. Reword both. Also fix a test docstring that still referenced the renamed _merge_rgb_options (now _unpack_rgb_options).
…e bullet list Review N2: the reworded **kwargs markdown table rows ran 122-364 chars (the 16-space docstring indent + fixed column widths push every row past the repo's 120-char limit, which allows no table-row exception). Convert the table to a bullet list that wraps at 120, and abbreviate the PointOverlay / ColorBar constructor lists (the full param lists live on the facade Args).
…ding branch Review N3: the cleopatra styling-params import in the NetCDF colour-kwargs builder ran on every plot even when no levels/style/hillshade was set. Push each import into the branch that actually builds a Contour / DataStyle, so a plain NetCDF plot never imports it.
…rOpts Review L1: NetCDF.plot exposes both the hoisted contour=/data_style= groups and the older ColorOpts(levels=/style=/hillshade=) bag. _build_render_kwargs unconditionally reassigned out['contour']/out['data_style'] from ColorOpts, silently discarding the caller's explicit group (contour=Contour(levels=[1,2,3]) + ColorOpts(levels=9) forwarded levels=9). Guard the ColorOpts-derived build on the key being absent so the explicit group wins, and add a regression test.
…loose style/hillshade Review L2: test_style_forwarded_to_mesh_render still passed loose style=/hillshade= and mocked _mesh_render, masking that the un-mocked UgridDataset.plot(style=...) now raises end-to-end. Convert it to the typed data_style=DataStyle group like its siblings.
Review L3: the [viz] floor is cleopatra >=0.30.0, but the pyramids.plot module docstring and its OptionalPackageDoesNotExist upgrade hint (plus the matching test) still said '>= 0.29', under-specifying the real requirement. Update those to 0.30, and drop the now -redundant '(cleopatra >= 0.28/0.29)' since-annotations from the feature docstrings (the 0.30 floor guarantees them). Also reword a stale render_array comment that still described the removed cbar_* folding (cbar_* are now rejected up front, not folded).
Review L4: ColorOpts.levels is converted to contour=Contour(...) in _build_render_kwargs before the animate drop runs, so the 'levels' entry in _ANIMATE_DROP_KWARGS never matches that path — it only caught a stray loose levels= kwarg, silently stripping it on the animate path while the static path raises. Remove it so a removed loose levels= is rejected consistently across modes.
Review N1: the ArrayGlyph reference-link line grew to 146 chars when the module path lengthened in the restructure (cleopatra.glyphs.gridded.array_glyph). Reflow the prose and drop the deep method anchor so the line fits the 120-char limit.
SonarCloud S9073: split 'assert a and b' composite assertions into separate asserts in the render-routing and the ColorOpts-precedence tests, so a failure pinpoints which half broke.
… test (SonarCloud python:S5778) SonarCloud S5778: the pytest.raises block also constructed FacetSpec/_points(), so more than the intended call could throw. Build them before the with-block so only nc.plot can raise inside it.
…mplexity (SonarCloud python:S3776) SonarCloud S3776: DatasetCollection.plot's Cognitive Complexity was 16 (> 15) after the render-group hoisting. Move the four RGB band-layout guards (issue #538) into a _validate_rgb_animation helper, dropping the branches out of plot. Behavior is unchanged (the warning stacklevel is bumped to 3 to keep pointing at the caller's plot() call).
…eopatra-0.29-migration # Conflicts: # src/pyramids/netcdf/plot_options.py
|
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
1 participant
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
Migrates pyramids onto cleopatra 0.30.0 (now released on PyPI). cleopatra 0.30 landed two breaking changes to
what pyramids consumes — a subpackage restructure and grouped render-parameter objects that replaced the
flat styling keywords — and this PR adopts both, aligns every pyramids plot facade with the new typed surface, and
removes the loose styling keywords so the typed group objects are the one way to style a plot.
1. Import migration (src + tests). The flat
cleopatra.*modules moved intoglyphs/{gridded,primitives,stats},styling/,basemap/subpackages and the package root re-exports nothing:array_glyph→glyphs.gridded.array_glyph(ArrayGlyph/FrameLabel/PointOverlay/PanelLabels);ColorBar→styling.colorbar;mesh_glyph/vector_glyph→glyphs.gridded.*;scatter_glyph/polygon_glyph→glyphs.primitives.*;colors/styles→styling.*;geo/reference/tiles→basemap.*.statistical_glyph.StatisticalGlyph→glyphs.stats.histogram_glyph.HistogramGlyph(renamed + moved).cleopatra.config.Configpath is unchanged.pyramids.plotlazy shim, every testimportorskip/@patchtarget, and the version gates(fixing the moved-symbol targets un-skipped ~360 tests that were silently skipping).
2. Typed render groups on every
.plotfacade. cleopatra replaced the flat styling keywords with typed groupobjects. pyramids re-exports them through
pyramids.plotand takes them as explicit, typed params:ColorScaling/Contour/CellValues/DataStyle/Classify/PanelLabels(alongsideColorBar/PointOverlay/FrameLabel/Basemap).Dataset.plot/DatasetCollection.plot/NetCDF.plot: hoistcolorbar/points/kind/titleand therender groups
color/contour/cells/data_style, typing the tail asUnpack[PlotKwargs]/Unpack[AnimateKwargs]where a single dict fits.UgridDataset.plot: hoistcolorbar/points/kind/title(mesh no-ops forpoints/kind) andcolor/contour/data_style(MeshGlyphhas no cell-value overlay).FeatureCollection.plot: hoistcolorbar/points/kind/titleacross both engines andcolor/contour/classify, forwarded to theScatterGlyph/PolygonGlyphplotcall onengine="cleopatra"(
colorbar→ geopandaslegend,title→ Axes title onengine="geopandas";points/kindare vectorno-ops).
[[tool.mypy.overrides]] module = "cleopatra.*"withfollow_untyped_imports = truelets mypy readcleopatra's inline types (it ships no
py.typed) soUnpack[PlotKwargs]resolves.3. Removed the loose styling keywords (breaking). With the grouped params now the upstream API, pyramids drops
its backward-compat translation shims — the loose forms are no longer accepted:
_build_grouped_render_specsand_migrate_deprecated_plot_specs. The forms cleopatra removed(
color_scale/gamma/bounds/midpoint/line_*/levels/label_kw/display_cell_value/num_size/background_color_threshold/style/hillshade/point_*/pid_*) now raise cleopatra's"moved onto a grouped parameter object" error.
cbar_*/ticks_spacingcolour-bar kwargs (which cleopatra still tolerates) are rejected bypyramids with a pointer to
colorbar=ColorBar(...)— pyramids exposes a single typed colour-bar surface.cbar_kwargs(the raw matplotlib dict) andadd_colorbarstay valid.ColorOptsbag is kept but rewired to emitcontour=Contour(levels=…)/data_style=DataStyle(style=…, hillshade=…); the paletted-raster path (feat(plot): render paletted rasters through their GDAL colour table (wire in _process_color_table) #913) now buildscolor=ColorScaling.boundary(bounds=…).4. Signature simplification — dropped the deprecated loose Sentinel kwargs (breaking). The four
Sentinel-imagery kwargs (
rgb/surface_reflectance/cutoff/percentile) were already deprecated on theplot facades in favour of the grouped
rgb_options=dict; the loose forms are now removed:Dataset.plot19 → 15 params;DatasetCollection.plot17 → 13. Both takergb_options=only.Dataset._merge_rgb_options(a 168-line loose+group merge withDeprecationWarningbranches) with alean
Dataset._unpack_rgb_options(rgb_options)— dict-only, same unknown-keyValueError.Analysis.plot,render_array) keep the four params; only the public facades lose theloose forms.
RasterBase.plot(the ABC) was tidied to match: loose params dropped,rgb_options=declared.5. Adopted the two behaviours the restructure unblocked (closes two issues).
ArrayGlyph.facetacceptscolorbar=ColorBar(feat(array_glyph): colorbar=ColorBar on facet (#256); collapse redundant returns cleopatra#271) — the facet path folds the colourbar like plot/animate; deleted the old
RENDER_ONLY_OVERRIDESfacet workaround.MeshGlyph.plotacceptscolorbar=ColorBar(feat(glyphs): accept colorbar=ColorBar on every glyph; validate label_location; full colorbar=True reset cleopatra#244) —plot_mesh_datatakes a typedColorBar(wasbool-only).Install source. cleopatra is the published
cleopatra[tiles]>=0.30.0via the[viz]extra (the temporarygit-pin used before the release is gone).
Issues
ColorBar. Enabled by feat(glyphs): accept colorbar=ColorBar on every glyph; validate label_location; full colorbar=True reset cleopatra#244:MeshGlyph.plot(colorbar=ColorBar(label=...))draws a labelled colour bar. ✅ verified.cbar_*workaround. Enabled by feat(array_glyph): colorbar=ColorBar on facet (#256); collapse redundant returns cleopatra#271:ArrayGlyph.facet(colorbar=ColorBar(label=...))returns aFacetGridwhose colour bar carries the caption.✅ verified.
Type of change
How Has This Been Tested?
tests/dataset/plot tests/netcdf/plot tests/ugrid tests/basemap tests/feature tests/dataset/collection→1665 passed, 3 skipped. The grouped-params migration and the loose-kwarg removal were both driven
test-first.
colorbar/points/kind/titleandcolor/contour/cells/data_style/classifyreach the render call across all facades; thecbar_*facet fold and the meshColorBar;title→Axes /colorbar→legend on the geopandas engine.cbar_*/point_*forms now raise; thepyramids.plotre-export resolves all 11specs. The obsolete folding / no-op / string-alias tests were deleted.
rgb_options=renders on both facades; theRasterBase.plotABCsignature-contract tests assert
rgb_optionsis present and the loose Sentinel params are gone. The obsoletergb/surface_reflectance/cutoffdeprecation & collision tests were deleted.mypyclean (0 issues); doctests on the changed modules pass.Migration notes (breaking)
Loose plot styling keywords are removed; pass the typed group objects (all re-exported from
pyramids.plot):color_scale/gamma/bounds/midpoint/line_threshold/line_scalecolor=ColorScaling.<variant>(...)levels/labels/label_kwcontour=Contour(...)display_cell_value/num_size/background_color_thresholdcells=CellValues(...)style/hillshadedata_style=DataStyle(...)scheme/k(vector)classify=Classify(...)point_color/point_size/point_label_*/pid_*points=PointOverlay(arr, ...)cbar_label/cbar_length/cbar_orientation/cbar_label_*/cbar_location/cbar_inside/cbar_box/cbar_tick_color/ticks_spacingcolorbar=ColorBar(...)rgb/surface_reflectance/cutoff/percentileonDataset.plot/DatasetCollection.plotrgb_options={"rgb": ..., "surface_reflectance": ..., "cutoff": ..., "percentile": ...}Checklist: