feat: classified rasters and scale-bar / north-arrow furniture - #363
Merged
Merged
Conversation
Wire the existing classification engine into the one glyph that draws a 2-D field. ArrayGlyph now mixes CLASSIFY_OPTIONS into its options and takes the same classify=Classify(scheme=..., k=...) parameter the scatter / vector / flow / polygon glyphs already expose, on plot, facet and animate. - A new _norm_cbar_and_ticks bridge routes ArrayGlyph's two norm call sites (_plot_im_get_cbar_kw, _plot_projected) through _prepare_classified_mapping when a scheme is set, building a BoundaryNorm over the array's finite cells and a stepped colorbar on the class edges; the continuous path is byte-for-byte unchanged when no scheme is set. - contourf draws its filled bands at the class edges when classified. - facet / animate resolve the class edges once over the whole stack, so every panel / frame shares one set of classes instead of re-binning its own slice. - A bad scheme rolls the whole group merge back, leaving no half-applied option on the sticky-options glyph; the conflict warning fires once. - scheme='categorical' stays rejected for a raster (its cells are a continuous field, not nominal labels). No new concept, type or dependency: classification stays numpy-only via styles.classify. Closes #351
Add the two remaining pieces of standard figure furniture cleopatra lacked, as free functions in a new cleopatra.styling.furniture module beside watermark.py: - add_scale_bar(ax, length, ...) draws a segmented bar with tick numbers and a caption, sized in the axes' own DATA units. - add_north_arrow(ax, *, rotation=..., style=...) draws a rotatable compass mark in the "arrow", "needle" or "rose" style. Both decorate an existing Axes and return the frameless inset axes they drew on, anchored in one of the four corners via stamp_mark's shared placement plumbing and reading like ColorBar (the same box / label_location / label_size vocabulary). They sit at a high zorder above the data and stay put across a dpi or limits change. The package boundary is the point: these are plain matplotlib artistry and own no geodesy. The length is a number in axis units and the label is a string the caller supplies; the rotation (grid convergence) is a number the caller supplies. No CRS, no ellipsoid, no pyproj import, no new dependency -- everything geodetic stays with the consumer that owns the CRS. The six GeoMixin glyphs gain thin add_scale_bar / add_north_arrow sugar next to add_tiles / add_features / add_labels; the free functions stay the API. Closes #352
Add a test that a bad scheme passed to ArrayGlyph.animate rolls the group merge back (leaving scheme unset) and re-raises, mirroring the existing plot rollback test and closing the one uncovered branch in the classify wiring.
The default add_scale_bar (lower corner, previously label_location="bottom") grew its tick numbers and caption downward, off the bottom of the axes and over the axis tick labels -- exactly the GeoMixin.add_scale_bar sugar default glyph.add_scale_bar(length). label_location now defaults to None (auto): a bar in a lower corner labels above the bar and one in an upper corner below, so the caption always faces the axes interior. Explicit "top"/"bottom" still honoured. Adds regression tests that the default call keeps all text within [0, 1] and that the auto side faces the interior for every corner.
…imate ArrayGlyph.plot / animate previously resolved the classification twice: an eager validation pass (warnings suppressed) plus a second resolve inside the render helper. That doubled the O(k n^2) Fisher-Jenks DP for natural_breaks / fisher_jenks, and made the scheme/scale conflict warning fire from the render helper -- one stack frame deeper than the pipeline glyphs -- so the shared stacklevel=5 mis-attributed it to array_glyph.py internals instead of the caller. Resolve _norm_cbar_and_ticks once, at plot/animate depth, and hand the (norm, cbar_kw, ticks) triple to _plot_im_get_cbar_kw / _plot_projected. Now classification runs a single time and the conflict warning attributes to the user call, matching the scatter/vector/flow/polygon glyphs. The single resolve still validates eagerly and rolls the group merge back on a bad scheme. Adds a warning-attribution regression test; updates the direct-call _plot_im_get_cbar_kw invalid-kind test to the new signature.
…trings Round-1 review follow-ups on the furniture module: - The caption is always drawn (it defaults to the length), but the backing box sized itself as if there were no text when ticks=False and no explicit label, leaving the caption hanging outside the panel. The box now always reserves caption room, with an extra row when tick numbers are present. - edge_color docstring claimed it set the block outline; the outline is drawn in color. Corrected. - needle/rose docstrings described a north/south split; the geometry splits each spike into two flanks. Corrected to match. Adds a test that the box vertical span contains the caption when ticks=False.
facet resolved the shared classification edges (classify_values over the whole stack) after _facet_axes had already created an owned figure but before the try/except that closes it on error. A raising scheme -- e.g. a spreadless stack, which classify rejects with "no spread" -- would then leak that figure. Move the edge resolution ahead of the figure creation so it can never leak. Adds a test asserting the resolution raises before _facet_axes is called.
…asters Round-1 review follow-ups: - Document on ArrayGlyph.plot that a classified render derives its classes from the data, so a caller vmin/vmax does not constrain them (pass explicit edges to pin boundaries), and that category_legend_kwargs is accepted but has no effect on a raster (categorical is rejected). - Add tests for the two remaining gaps the review flagged: a loose scheme= keyword on the ArrayGlyph constructor still raises the migration error, and a classified raster with NaN cells bins only its finite values.
The N3 box change added a below-the-bar (sign < 0) branch that no box test exercised, dropping furniture.py to 99%. Add a bottom-caption box test to restore 100% line + branch coverage.
…lues facet(classify=...) crashed when any single panel slice was entirely masked / NaN: each panel re-invokes classify with the stack-wide explicit edges, but classify rejected an all-non-finite input before checking that the scheme was an explicit edge sequence -- for which the values are irrelevant. A realistic geospatial case (a cloud-covered time slice) thus aborted the whole facet, while animate handled it fine. Move the no-finite guard into the named-scheme branch: an explicit edge sequence now bins even empty / all-non-finite values (the edges are given). Named schemes still require finite data. Tests cover both.
A data_style preset owns the colour mapping and takes a separate render path, so a co-passed classify was silently dropped (final norm Normalize, no norm from the scheme) -- inconsistent with the scheme/color_scale conflict warning and the adjacent points / cell-value warnings on that path. plot and animate now warn that classify is ignored with a data_style preset. Documented on the classify param.
…ecedence Round-2 review test additions: - explicit edges classify all-non-finite values without raising (M1); - facet with a fully-masked panel shares stack-wide edges (M1); - data_style preset over classify warns (S2); - classify through the projection render path (N1); - a later animate frame keeps the discrete norm (N2).
…ero check SonarCloud S3776 (cognitive complexity 20 > 15) and S1244 (float == on the x-range guard) on add_scale_bar. Extract the block-drawing (_draw_bar_segments) and the tick/caption drawing (_label_scale_bar) into helpers, dropping the function well under the complexity limit, and replace the "data_range == 0.0" guard with a truthiness check. No behaviour change (72 furniture tests green).
…cations Clear the mechanical SonarCloud test-file findings on the code this PR added: - S9073: split each "assert A and B" into separate assertions; - S5778: hoist the Classify / FacetLayout construction out of pytest.raises blocks so only the intended call can throw; - S9088: hoist the Classify / DataStyle construction out of the pytest.warns block so only plot() can emit the warning; - S9083: drop the empty parentheses from the TestArrayGlyphScheme.ramp fixture.
After merging #362 (hatch encoding), classify and Contour(fill=False) both live in the contourf path. fill=False renders colors="none" (an unfilled hatch-only overlay), so a co-passed classify draws no class colours even though it still sets the band edges and a stepped colorbar. Warn that the class colours are not drawn in that case, consistent with the color_scale / data_style conflict warnings. Test covers the combination.
SonarCloud S107: add_scale_bar had 14 parameters (> 13). Group the twelve presentation options (label, location, pad, height, segments, ticks, color, edge_color, label_location, label_size, box, zorder) into a frozen ScaleBar dataclass, mirroring FacetLayout / ColorBar, so the call is add_scale_bar(ax, length, ScaleBar(...)) -- three parameters. BREAKING CHANGE: add_scale_bar and GeoMixin.add_scale_bar no longer take the loose presentation keywords; pass a ScaleBar spec instead. length and ax stay direct arguments.
…mation SonarCloud S107: animate had 16 parameters (> 13), tripped by the classify addition. Group the four animation-specific playback options -- interval, frame_label, cell_value_text_colors, data_getter -- into a frozen Animation dataclass, passed as animate(playback=Animation(...)). The render / colour options (color, contour, cells, classify, data_style, colorbar, ...) stay their own arguments, so the signature drops to 12 named parameters plus kwargs. BREAKING CHANGE: ArrayGlyph.animate no longer takes the loose interval / frame_label / cell_value_text_colors / data_getter keywords; pass them on an Animation via playback=. time and points stay direct arguments.
For symmetry with ScaleBar, group add_north_arrow presentation options (location, pad, size, style, label, color, edge_color, label_size, box, zorder) into a frozen NorthArrow dataclass: add_north_arrow(ax, rotation, NorthArrow(...)). rotation stays a direct argument (the caller-supplied grid convergence, like length on the scale bar). GeoMixin sugar, test call sites and docs migrated. BREAKING CHANGE: add_north_arrow and GeoMixin.add_north_arrow no longer take the loose presentation keywords; pass a NorthArrow spec instead. ax and rotation stay direct arguments.
…blocks Grouping add_scale_bar / add_north_arrow / animate params into ScaleBar / NorthArrow / Animation specs put the spec construction inside pytest.raises blocks in the validation tests, so SonarCloud S5778 flagged two throwing invocations per block. Hoist each spec to a local before the with-block, so only the intended call can throw. Covers the ten new-code S5778 findings; the pre-existing ones on unchanged lines are out of scope.
The docs build executes the notebooks, and six used the old ArrayGlyph.animate signature (interval= / frame_label= / cell_value_text_colors= / data_getter=) that the Animation grouping replaced, failing mkdocs CI. Move those keywords onto playback=Animation(...) and import Animation where needed. Notebooks stay output-stripped.
|
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
Two self-contained visualization features, each wiring or adding capability with no new dependency.
#351 —
classify=onArrayGlyph. The classification engine (styles.classify, theClassifyparamsobject,
Glyph._prepare_classified_mapping, the stepped colorbar and the conflict warning) already existed butthe one glyph that draws a 2-D field could not reach it.
ArrayGlyphnow mixesCLASSIFY_OPTIONSinto itsoptions and
plot/facet/animateaccept the sameclassify: Classify | Noneparameter the four pipelineglyphs (scatter / vector / flow / polygon) already expose:
A new
_norm_cbar_and_ticksbridge routes ArrayGlyph's two norm sites through_prepare_classified_mappingwhen a scheme is set — aBoundaryNormover the array's finite cells and astepped colorbar on the class edges — while the continuous path is byte-for-byte unchanged when no scheme is
set.
facet/animateresolve the class edges once over the whole stack so panels / frames share one set ofclasses. A bad scheme rolls the group merge back;
scheme="categorical"stays rejected for a raster (its cellsare a continuous field, not nominal labels). numpy only — no mapclassify, no new dependency.
#352 — scale bar and north arrow. A new
cleopatra.styling.furnituremodule, besidewatermark.py, addsthe two remaining pieces of standard figure furniture as free functions that decorate an
Axesand return theframeless inset they drew on:
They reuse
stamp_mark's corner-placement plumbing andColorBar'sbox/label_location/label_sizevocabulary. The package boundary is the point: they own no geodesy —
lengthis a number in axis data units,labelandrotationare supplied by the caller. No CRS, no ellipsoid, nopyprojimport, no new dependency —everything geodetic stays with the consumer that owns the CRS. The six
GeoMixinglyphs gain thinadd_scale_bar/add_north_arrowsugar next toadd_tiles/add_features/add_labels.Issues
classify=onArrayGlyph.plotso a raster can be drawn in quantile, equal-interval or natural-breaks classes #351 — acceptclassify=onArrayGlyph.plot/facet/animate(classified raster)Type of change
Check relevant points.
How Has This Been Tested?
Run against the external uv env (
C:\python-environments\uv\cleopatra) withPYTHONPATH=<worktree>/src.tests/test_classify.pyinvertstest_array_glyph_rejects_schemeto an accept-and-rendertest, adds
ArrayGlyphto the accept-scheme parametrize, and adds a fullTestArrayGlyphSchemeclass(discrete classes, stepped colorbar on the class edges, raw values preserved,
scheme=Noneregression,contourf, categorical-rejected, rollback, and shared classes across facet panels / animation frames). New
tests/test_furniture.pycovers both artists at 100% line + branch — corner placement in all fouranchors, segment geometry, tick numbers / caption placement, the backing box, zorder above the data,
stability across a
set_xlimchange, the three arrow styles and rotation, validation errors, and theGeoMixinsugar.test_classify.py(80),test_array_glyph.py+test_params.py(633),test_furniture.py(63),test_geo.py(131); consolidated re-run 280 passed. Doctests green forarray_glyph.py(58),furniture.py,params.py,styles.py.ruff format --checkandruff checkcleanon every changed file (pinned pre-commit ruff 0.15.22).
Checklist:
docs/change-log.mdis auto-generated by commitizen from theConventional Commit history)