Skip to content

chore(plot): drop the facet loose-cbar special-case once cleopatra#256 lands #934

Description

@MAfarrag

Context

pyramids folds the loose plot kwargs into the typed specs at the render_array boundary — cbar_*
colorbar=ColorBar(...), point_*/pid_*points=PointOverlay(...), a dict basemap → Basemap(...) — so
cleopatra only ever sees the typed form (PR #930). The facet path is the one exception: it is skipped because
cleopatra's ArrayGlyph.facet does not accept colorbar=ColorBar (only the loose cbar_* kwargs, like the mesh
glyph), so faceting keeps the loose forms. Filed upstream as serapeum-org/cleopatra#256.

Related: #926, #930 (the fold), #933 (the mesh half), cleopatra#256.

Problem

Because of the cleopatra facet gap, render_array carries a facet special-case:

  • src/pyramids/dataset/_plot_helpers.py — the if mode != "facet": guard that skips
    _migrate_deprecated_plot_specs, plus the cbar_* kept in RENDER_ONLY_OVERRIDES so the loose kwargs still
    reach cleo.facet and render.

The consequence: faceted plots stay on the deprecated loose cbar_* with no migration nudge, and the plot API is
inconsistent (plot/animate fold to the typed spec; facet does not).

Blocked by

serapeum-org/cleopatra#256 — "accept colorbar=ColorBar on ArrayGlyph.facet (+ warn on loose cbar_*)".
Do this once that lands in a cleopatra release pyramids adopts.

Affected locations

File Symbol Change
src/pyramids/dataset/_plot_helpers.py render_array drop the if mode != "facet": guard; drop cbar_* from RENDER_ONLY_OVERRIDES; remove the TODO(cleopatra#256) comment
tests/dataset/plot/test_new_render_params.py test_deprecated_cbar_kwarg_renders_on_the_facet_path update: facet now folds cbar_*ColorBar like plot/animate

Proposed Solution

Once cleopatra#256 ships and the [viz] floor is bumped:

  • Remove the mode != "facet" guard so _migrate_deprecated_plot_specs runs for facet too (folds cbar_*
    ColorBar, point_*PointOverlay).
  • Remove the cbar_* entries from RENDER_ONLY_OVERRIDES (leaving only {"kind"}) — no longer needed once the
    loose kwargs are folded before the split.
  • Rework the facet test to assert the fold (a loose cbar_* on facet → colorbar=ColorBar + DeprecationWarning).
  • Delete the TODO(cleopatra#256) breadcrumb.

Effort Estimate

Size: S — a guard/override deletion + one test rework, gated on the cleopatra bump.

Definition of Done

  • cleopatra#256 is released and pyramids' [viz] floor includes it
  • the facet path folds the loose cbar_* into ColorBar (no mode != "facet" special-case)
  • RENDER_ONLY_OVERRIDES is back to {"kind"}; the TODO(cleopatra#256) comment is gone
  • the facet test asserts the fold; the full plot suite stays green

Metadata

Metadata

Assignees

No one assigned

    Labels

    dependenciesPull requests that update a dependency fileplotting

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions