Tweak AnnotationBbox coords specification.#26069
Conversation
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()?
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. |
|
Does this need an API change note? |
tacaswell
left a comment
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.
|
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. |
4 participants
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 thanAnnotationBbox(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 beAnnotationBbox(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 toAnnotationBbox(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