Allow outline_color to accept an obs column#683
Open
timtreis wants to merge 14 commits into
Open
Conversation
Symmetric with `color`: a non-color string passed to `outline_color` is now resolved as a column on the annotating table (or a different table via the new `outline_table_name` on the render params) and the outline is rendered per-shape / per-label via the same `palette` / `cmap` / `na_color` used for fill. Shapes (matplotlib + datashader) and labels are covered. When both fill and outline are categorical columns, two stacked legends are drawn, auto-positioned to avoid overlap. The 2-outline form remains literal-only; combining it with a column outline raises.
Correctness: - Labels: compute outline color vector before fill's rasterize/groups masks so the same masks can be applied to outline vectors without IndexError. - Datashader: ride outline column under a dedicated internal column name so it never overwrites the fill column when fill and outline share a key. - `_map_color_seg`: drop the unreachable categorical-dtype branch on the outline vector; only the (Categorical source, hex-string vector) and continuous-numeric paths are reachable for this call site. - Continuous datashader outline: thread `cmap_params.norm` through `_apply_ds_norm` + `span=` so explicit vmin/vmax takes effect. - Continuous outline column: stack a second colorbar via `colorbar_requests` instead of dropping the legend entirely. - Outline cross-table join: use `sdata_filt` (cs-filtered) instead of the original `sdata`; pad/truncate outline vectors when fill+outline tables annotate different row counts. Typing: - Widen `_color_vector_to_rgba.color_vector` to `Any | None`. Tests added: - groups-filter + data-driven outline (shapes + labels). - column collision (`outline_color="red"` shadowing an obs column) raises. - datashader continuous outline. - continuous outline produces a colorbar. - labels outline-only mode with column outline.
Reuse / dedup: - Promote inline `reduction_function_map` (duplicated between `_render_ds_outline_by_column` and `_datashader_aggregate_with_function`) to a module constant `_DS_REDUCTION_FUNCS`. - Extract `_align_outline_vector_to_length` for the pad/truncate logic shared by shapes (cross-table) and labels (rasterize alignment). - Extract `_append_outline_colorbar` for the continuous-outline `ColorbarSpec` construction shared between `_add_legend_and_colorbar` and `_render_labels`. Encapsulation: - Promote the outline ride-along column name `"__sdp_outline_col__"` to a named module constant `_OUTLINE_INTERNAL_COL` in `_datashader.py`. - Move the column attachment inside `_render_ds_outline_by_column` (via `.assign()`) so the rasterizer element isn't mutated by the caller. Performance: - Drop the unconditional `astype(float, copy=True)` in `_get_collection_shape`'s outline path: reuse the caller's array when it's already float (saves N×32 bytes per render).
- Drop try/except wrapping the fig.canvas.draw() + get_window_extent() measurement in `_add_outline_legend`. The existing `anchor_y > 0` threshold already handles the degenerate-bbox case; raising on a real matplotlib backend failure is the correct behavior. - Replace narrowing `assert ... # noqa: S101` with `typing.cast` in `_add_legend_and_colorbar`. - Drop the `hasattr(outline_color_vector, "__getitem__")` defensive guard in `_render_shapes`' groups-mask block — by that point the vector is always an array/Series/Categorical. - Document the in-place mutation of `(N, 4)` RGBA arrays passed as `outline_color` to `_get_collection_shape`. - Drop the `na_color` parameter from `_align_outline_vector_to_length`: pad with NaN regardless of source-vector type so downstream code maps padded rows to na_color via the cmap. Categorical padding now skips the unused na-hex allocation. Tests added: - `_align_outline_vector_to_length`: no-op, truncate, continuous pad, categorical pad. - Cross-table render: fill column in table A, outline column in table B. - Datashader continuous outline respects an explicit `Normalize`.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #683 +/- ##
==========================================
- Coverage 77.81% 77.48% -0.33%
==========================================
Files 11 11
Lines 3700 3958 +258
Branches 879 941 +62
==========================================
+ Hits 2879 3067 +188
- Misses 491 540 +49
- Partials 330 351 +21
🚀 New features to boost your workflow:
|
timtreis
changed the title
outline_color to accept an obs column (closes #681)outline_color to accept an obs column
22 tests → 9 (5 are parametrize expansions). Dropped tests that: - assert on internal render-params dataclass fields rather than rendered output (`test_outline_color_column_sets_render_params`, `*_literal_still_color`, `test_labels_outline_color_sets_render_params`, `*_literal_still_works`, `*_outline_only_mode`). - duplicate coverage already exercised by other tests (four `_align_outline_vector_to_length` unit tests are now covered by the cross-table and groups-filter render tests). - lock implementation details rather than behavior (`test_outline_color_continuous_requests_colorbar` introspects ColorbarSpec). Folded `test_outline_color_column_continuous`, `*_column_datashader`, `*_datashader_continuous`, `*_ds_continuous_respects_explicit_norm` into a single parametrized `test_outline_color_column_renders[col,method,norm]`.
- Extract `_join_table_for_element` (utils.py) to wrap the index-name workaround for scverse/spatialdata#1099 around `join_spatialelement_table`. Use from both the fill-side join in `_render_shapes` and the outline cross-table join. The outline path previously omitted the workaround and would have hit the same upstream bug on tables with name-colliding obs indices — this is now patched. - Extract `_apply_mask_to_outline_vectors`: replaces 5-line duplicate blocks in `_render_shapes` (groups branch) and `_render_label` (rasterize + groups branches). - Extract `_decorate_outline`: the "categorical legend | continuous colorbar" dispatch was duplicated between `_add_legend_and_colorbar` and the end of `_render_label`. - Extract `_make_continuous_mappable`: the `vmin == vmax → ±0.5` fallback used by both `_build_ds_colorbar` and `_append_outline_colorbar`. - Vectorize the categorical color-map loop in `_map_color_seg`'s outline branch: `for cat_idx in range(K): np.where(cat_codes == cat_idx)` → one `np.unique(cat_codes, return_index=True)` pass. Net: -117 + 160 LOC across the three source files; one latent correctness gap fixed; ~K-times fewer Python loop iterations on the labels-with-categorical-outline hot path.
…681) Dropped non-visual tests whose only assertion was "did not raise": - `test_outline_color_as_obs_column_does_not_raise` - `test_outline_color_column_renders` (5 parametrize cases) - `test_outline_color_column_stacked_legends` (bbox non-overlap was a proxy for "legends are visibly placed correctly") - `test_labels_outline_color_obs_column_does_not_raise` Added 6 visual `test_plot_*` tests gated against committed reference PNGs: - TestShapes.test_plot_outline_color_by_categorical_obs (matplotlib) - TestShapes.test_plot_outline_color_by_continuous_obs (matplotlib) - TestShapes.test_plot_outline_color_by_categorical_obs_datashader - TestShapes.test_plot_outline_color_by_continuous_obs_datashader - TestShapes.test_plot_fill_and_outline_both_obs_columns (stacked legends) - TestLabels.test_plot_outline_color_by_categorical_obs_labels Kept the 5 non-visual tests for error paths and alignment regressions (2-outline+column rejection, color/column collision, groups+outline IndexError regression x2, cross-table join exercise) — these gate behaviors that visual tests can't catch reliably. Reference images will be generated on first CI run and committed from the CI artifact, per project convention.
For `render_labels` with `outline_color="<obs col>"` but no `color=` (or fill column without a table), the labels render previously derived `instance_id` from `np.unique(label.values)` and pulled the outline vector from the table in table-row order. The two were positionally misaligned: position 0 of `instance_id` is the background label 0, position 0 of the outline vector is the first table row. Result: every label was colored by the wrong row's category. With an alternating c1/c2 column on a fixture where the table doesn't cover all label values, this collapsed to roughly all-c2 in the rendered image. Fix: promote `outline_table_name` to drive instance_id derivation when fill has no table. The outline vector then aligns to `instance_id` via the table's `instance_key`, matching the existing fill-with-table path.
`transformed_element.assign(col=series)` aligns by index. When the post-inner-join element carries a non-contiguous index (e.g. [0, 2, 5, ...]) but the outline source vector is built with a default RangeIndex(0..n-1), rows whose indices don't intersect get NaN. Those NaNs are then lifted to the `ds_nan` sentinel by `_inject_ds_nan_sentinel`, and one polygon ends up rendered as `na_color` instead of its real category — visible as a gray outline among otherwise correctly-colored polygons on the Linux CI runners. The matplotlib path is unaffected because it goes through `_color_vector_to_rgba`, which operates positionally. Fix: assign positionally via `df[col] = pd.Categorical(...)` (which uses position-based assignment for ExtensionArray-typed RHS), on an explicit copy so the caller's dataframe is not mutated.
Drop the hardcoded `outline: <col>` prefix on the outline categorical legend and the outline continuous colorbar. Behavior: - Single colored channel (fill OR outline) → no title on the legend. - Both colored channels → auto-title `fill` / `outline` so the user can tell which is which. - New kwargs on `pl.show()`: `legend_title` (fill) and `outline_legend_title` (outline). Either overrides the auto-title; pass an empty string to suppress. The fill legend now also supports a title: scanpy's `_add_categorical_legend` doesn't accept one, so it's set post-hoc on `ax.get_legend()`. Stale visual reference images are removed in this commit so CI regenerates them for the new title behavior.
Scanpy's `_add_categorical_legend` anchors the fill legend at `bbox_to_anchor=(1, 0.5)` (vertically centered). With a second legend added below for outline-by-column, the two looked unbalanced: fill at the vertical center, outline below it, with a visible gap. Reposition the fill legend to `(1.02, 1.0) loc='upper left'` whenever an outline legend will be drawn alongside, so the two stack from the top with consistent spacing. Single-legend rendering is unchanged (no second legend, no reposition). The Shapes_fill_and_outline_both_obs_columns reference is regenerated on CI.
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
2 participants
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.
Summary
outline_coloronrender_shapesandrender_labelsnow accepts an obs column name (mirroringcolor/col_for_color). Outlines are drawn per-shape / per-label via the samepalette/cmap/na_colorused for the fill.cvs.line+ds.by/ numeric reduction), and labels (_map_color_segextended for column-driven contours).ColorbarSpecis appended via the existingcolorbar_requestsflow.outline_color=("a", "b")) remains literal-only; combining it with a column outline raisesValueError.Closes #681