Wire scalebar_dx/scalebar_units into pl.show()#648
Merged
Conversation
The scalebar machinery (ScalebarParams, _get_scalebar, ScaleBar import) was
present but unreachable: show() did not declare or forward scalebar_dx /
scalebar_units, so any user attempt raised TypeError. The downstream
ScaleBar(dx=[...], units=[...]) call in _add_decorations_to_ax also passed
broadcast lists where ScaleBar requires scalars, and ran once per render
layer (so multi-layer plots would have stacked duplicates).
- Add scalebar_dx, scalebar_units (default "um"), and scalebar_params
(kwargs dict, mirroring colorbar_params) to show()'s signature.
- Centralize drawing in _draw_scalebar(ax, params, panel_idx); call it once
per axis at the tail of show()'s panel loop.
- Drop the broken scalebar block + scalebar_dx/units kwargs from
_add_decorations_to_ax; drop the now-unused scalebar_params arg from
_render_{shapes,points,images,labels} and _add_legend_and_colorbar.
- Validate scalebar_dx/units/params types and sign in
_validate_show_parameters.
- Add 9 non-visual regression tests + 2 visual tests (default and styled).
timtreis
changed the title
Cover three orthogonal scalebar knobs that users routinely tune: - frameon=False (no surrounding box) - length_fraction + pad + border_pad (compact footprint) - fixed_value + label (pin bar length, override displayed text) Also drops a transient explanatory comment block above the non-visual scalebar tests; the test names and docstrings are self-explanatory.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #648 +/- ##
==========================================
+ Coverage 76.39% 76.83% +0.43%
==========================================
Files 11 11
Lines 3237 3276 +39
Branches 759 773 +14
==========================================
+ Hits 2473 2517 +44
+ Misses 466 458 -8
- Partials 298 301 +3
🚀 New features to boost your workflow:
|
Mirrors the colorbar_params / scalebar_params escape-hatch pattern. The five flat legend_* kwargs continue to work unchanged; legend_params is purely additive sugar so existing scripts and the wider scverse muscle memory (legend_fontsize, legend_loc, na_in_legend) keep functioning. Inside the dict, keys use matplotlib-native bare names (loc, fontsize, fontweight, fontoutline, na_in_legend) rather than the prefixed flat names — the dict label already provides the namespace, and bare keys match matplotlib.legend.Legend documentation that users are most likely to copy from. Unknown keys raise ValueError to surface typos early. When the same option is set both as a flat kwarg and inside the dict, the dict wins. No DeprecationWarning is emitted; this is Phase 1 of a potential multi-phase migration but does not commit to deprecating the flat kwargs. Adds 7 tests covering dict form, override precedence, None no-op, and validation (parametrized x4 over bad types and unknown keys).
matplotlib.legend.Legend natively uses 'loc' while Figure.colorbar and matplotlib_scalebar both use 'location', which would force users to remember a different key name for legend_params than for colorbar_params and scalebar_params. We accept both spellings so the three escape-hatch dicts read consistently; 'location' is documented as canonical and wins when both are passed. Inline comments at the merge site and the validation whitelist explain the matplotlib quirk so future maintainers don't 'simplify' the alias away.
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
pl.show()did not declare or forwardscalebar_dx/scalebar_units, so any attempt raisedTypeError. The downstreamScaleBar(dx=[...], units=[...])call in_add_decorations_to_axalso passed broadcast lists whereScaleBarrequires scalars, and ran once per render layer (so multi-layer plots would have stacked duplicates).show():scalebar_dx: float | None = None,scalebar_units: str = "um",scalebar_params: dict | None = None(kwargs dict mirroring the existingcolorbar_paramsidiom; names match scanpy/squidpy/matplotlib_scalebar)._draw_scalebar(ax, params, panel_idx)helper invoked once per axis at the tail ofshow()'s panel loop. Removes the broken scalebar block from_add_decorations_to_axand the now-unusedscalebar_paramsargument from_render_{shapes,points,images,labels}and_add_legend_and_colorbar.UX rationale
SpatialData coordinate systems carry no unit metadata, so automatic detection of
dxis impossible (unlike LazySlide, which reads MPP from WSI metadata). User-supplieddxis the only option — same constraint scanpy/squidpy operate under.scalebar_paramsinstead of carving out individual styling kwargs keeps the signature tight and matchescolorbar_paramsalready in the same function.Closes #614.