Repository navigation
Tweak AnnotationBbox coords specification. - #26069
ksunden merged 1 commit into
Conversation
Sorry, something went wrong.
In AnnotationBbox, it makes sense to allow passing xycoords and boxcoords positionally, because (similarly to annotate()) something like `AnnotationBbox(box, xy1, xy2, "axes transform", "offset points")` (for example) actually reads better than `AnnotationBbox(box, xy1, xy2, xycoords="axes transform", boxcoords="offset points")` once you've seen this often enough and learn that the positional order is just (annotated_xy, box_xy, annotated_coords, box_coords). (The alternative that's fully explicit would be `AnnotationBbox(box, xy=xy1, xybox=xy2, xycoords=..., boxcoords=...)` but even that doesn't read too well because the xy kwargs names don't rhyme exactly with the coords names; or one could reorder the args to `AnnotationBbox(box, xy=xy1, xycoords=..., xybox=xy2, boxcoords=...)`, but that's really a mouthful and doesn't help API usability.) So let's just allow passing the coordinate systems positionally (undoing the change in Matplotlib 3.6 that made them kwonly). The following kwargs do remain kwonly, though. In demo_text_path: xycoords defaults to "data" so doesn't need to be specified; xybox is not specified so boxcoords is not needed.
There was a problem hiding this comment.
Sounds reasonable.
The better API would be grouping positions and coords together: AnnotationBbox(box, xy1, "axes transform", xybox, "offset points"). But that ship has sailed. While we might have considered this for the rather seldom used AnnotationBbox, we cannot make that change to annotate(), and having different orders between different times of annotations is a non-starter. So this is the best we can go to.
Speaking of which, should we equally limit the use of positional arguments in annotate()?
Sorry, something went wrong.
Currently the signature is def annotate(self, text, xy, xytext=None, xycoords='data', textcoords=None,
arrowprops=None, annotation_clip=None, **kwargs):I guess you could make annotation_clip kwonly; I'm not really sure it should really be done for arrowprops, because in practice it will often be self-explanatory in the code if you really end up passing all these positionally (e.g. annotate("string", xy, "data", xy1, "offset fontsize", {"arrowstyle": "->"}): writing arrowprops={"arrowstyle": "->"} seems repetitive, and note that arrowprops usually contains an arrowstyle key). |
Sorry, something went wrong.
|
Does this need an API change note? |
Sorry, something went wrong.
tacaswell
left a comment
There was a problem hiding this comment.
I'm 50/50 on this needing an API change note. If anyone feels "no", merge away.
Sorry, something went wrong.
|
The notes added in the original expiration (#24984) contain the following already, but do not spell out each individual artist's cut off:
Given that incorrect info is not in the change notes, but the general idea is already (and this is a scaled back version of the deprecation expiring in 3.8 which will have been warning users already) I don't think an additional change not is needed. |
Sorry, something went wrong.
In AnnotationBbox, it makes sense to allow passing xycoords and boxcoords positionally, because (similarly to annotate()) something like AnnotationBbox(box, xy1, xy2, "axes transform", "offset points") (for example) actually reads better than
AnnotationBbox(box, xy1, xy2, xycoords="axes transform", boxcoords="offset points") once you've seen this often enough and learn that the positional order is just (annotated_xy, box_xy, annotated_coords, box_coords). (The alternative that's fully explicit would be
AnnotationBbox(box, xy=xy1, xybox=xy2, xycoords=..., boxcoords=...) but even that doesn't read too well because the xy kwargs names don't rhyme exactly with the coords names; or one could reorder the args to AnnotationBbox(box, xy=xy1, xycoords=..., xybox=xy2, boxcoords=...), but that's really a mouthful and doesn't help API usability.)
So let's just allow passing the coordinate systems positionally (undoing the change in Matplotlib 3.6 that made them kwonly). The following kwargs do remain kwonly, though.
In demo_text_path: xycoords defaults to "data" so doesn't need to be specified; xybox is not specified so boxcoords is not needed.
PR summary
PR checklist