Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Tweak AnnotationBbox coords specification.#26069
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
In AnnotationBbox, it makes sense to allow passing xycoords andboxcoords positionally, because (similarly to annotate()) something like`AnnotationBbox(box, xy1, xy2, "axes transform", "offset points")` (forexample) 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 orderis just (annotated_xy, box_xy, annotated_coords, box_coords). (Thealternative 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'trhyme 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). Thefollowing kwargs do remain kwonly, though.In demo_text_path: xycoords defaults to "data" so doesn't need to bespecified; xybox is not specified so boxcoords is not needed.
timhoffm left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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 usedAnnotationBbox, we cannot make that change toannotate(), 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 inannotate()?
Currently the signature is defannotate(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? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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. |
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