Refactor autoshapes to use shape.label for vline/hline/vrect/hrect#5444
Refactor autoshapes to use shape.label for vline/hline/vrect/hrect#5444nochinxx wants to merge 13 commits intoplotly:mainfrom
Conversation
|
Wow, fantastic PR! This is an exciting refactor and would be a great add to Plotly.py. Let us know when you need help reviewing. |
…el_from_legacy_annotation_kwargs(kwargs) to format kwargs into shape.label
…bel (prep for shape.label refactor)
…position normalizer -refactor(vline): emit a single labeled shape; map legacy annotation_position to label.textposition
…notation positions to the shape’s label.textposition.
Migrate add_hrect from annotation to shape/label#
c9062f1 to
1010346
Compare
Thanks! I’ve updated the branch and this is now ready for initial review. |
|
Thanks @nochinxx for the PR! I'll review within the next few days. |
|
@nochinxx Thanks for your work on this. It does seem that in order to do this "right", we'll have to change the API for shape labels, and support both APIs in parallel until the next major version, as you've done in this PR. Before I do a more detailed review, could you update the PR description to follow the template and include a few screenshots of shape labels created using the new API? |
|
I'll work on that. Thanks |
…layout.shape.label
|
@emilykl I've updated the PR description to follow the template, it now includes a link to the motivating issue, a description of the changes, demo code with screenshots of shape labels created using the new API (for all four helpers, plus a backward-compat example), and a testing strategy section. Let me know if you'd like me to rebase onto the latest main before you dig in, or if anything else needs adjusting. Thx |
Link to issue
(#5373)
Closes #(5373)
Description of change
This PR begins migrating the autoshape helpers (
add_vline,add_hline,add_vrect,add_hrect) to use the nativelayout.shape.labelAPIintroduced in Plotly.js, while keeping full backward compatibility with the
existing
annotation_*kwargs.Demo
New API —
label=parameterVertical line with a label at the top
Horizontal line with a label on the right
Vertical rect with a label inside
Horizontal rect with a label
Legacy API — annotation_text= still works (backward compat)
Testing strategy
Ran existing autoshapes test suite to verify no regressions:
python -m pytest -q tests/test_optional/test_autoshapes/test_annotated_shapes.pyAdded three new test cases:
test_legacy_annotation_text_also_sets_shape_label— verifiesannotation_text=is mirrored toshape.label.textwhile also producing alayout.annotationsentry (backward compat).test_explicit_label_takes_precedence_over_legacy_annotation_text— verifies that an explicitlabel=dict(text=...)wins overannotation_text=on the shape label (annotation still gets the legacy value).(Existing tests that assert on
layout.annotationsall continue to pass because the annotation path is still active.)Additional information (optional)
What changed:
label=parameter is now respected on all four autoshape helpers,forwarding a
shape.labeldict directly to the underlying shape.annotation_*kwargs are translated toshape.labelfields on abest-effort basis (text, font, textangle) so existing code benefits without
any changes.
annotation_positionis mapped tolabel.textpositionwhere the mappingis unambiguous (e.g.
"top"→"end"for vlines,"left"→"start"for hlines,
"top left"→"top left"for rects).FutureWarningis emitted whenannotation_*kwargs are used,encouraging migration to
label={...}.layout.annotationsentriesare still created, so all current tests and user code continue to work.
Why this matters:
Using the native
shape.labelavoids the coordinate arithmetic thatadd_vlinepreviously did to place its annotation. That arithmetic broke ondatetime x-axes (see #3065) because it mixed
datetimeandfloattypes.The
shape.labelpath has no such arithmetic — Plotly.js handles placemententirely.
This is an intentionally incremental step. The existing annotation creation
is still in place; removing it (and the deprecation warning) is planned for
a follow-up PR once the
shape.labelpath is confirmed to be sufficient.The
annotation_bgcolor / annotation_bordercolorfields are not supportedby
shape.labeland are explicitly ignored with aFutureWarningGuidelines