matplotlib
Add bbox support to quiverkey
-This change was necessary to resolve the issue #29704. -It is now possible to add a bbox (bounding box) around the entire quiverkey label — including both the arrow and the text — using the bbox keyword argument. Previously, bounding boxes could only be added manually or were limited to wrapping just the text, not the full key. This enhancement allows users to apply background color, border, and padding around the complete key for better visibility and styling.
PR checklist
- [ N/A] "closes #0000" is in the body of the PR description to link the related issue
- [ ] new and changed code is tested
- [ ] Plotting related features are demonstrated in an example
- [N/A ] New Features and API Changes are noted with a directive and release note
- [ ] Documentation complies with general and docstring guidelines
I will resolve the problems with newlines and blankspaces but, the problems with Tests/ Python 3.12, it's not because of something I did right?
I'm not really sure it's actually a good idea to add bbox support in each artist class one at a time. Instead, it may be more viable to have something like FancyBboxPatch.around_artists(list-of-artists, **kwargs); or at least have bbox support in the base artist class, relying on the generic get_window_extent method to figure out how big to draw the bbox?
Hm ok, i understand your point, but I did what I understood throught my conversation with the developers on the tab of the issue (I did show the API, etc). Maybe I understood wrong. @story645, @rcomer, what do you think it's the best? Do I have to change my aproach?
I'm not really sure it's actually a good idea to add bbox support in each artist class one at a time.
Which artists would it make sense to have a bbox for?
Do I have to change my aproach?
Depends - if a method is added to fancy box then yes, if the implementation is in base artist than you'll move the solution into the artist class but you still need to propose the arguments and show examples.
attn @timhoffm
Hm ok, i understand your point, but I did what I understood throught my conversation with the developers on the tab of the issue (I did show the API, etc).
Please understand that not all developers read all discussions at the same time nor do we all necessarily agree immediately on the best way to implement a feature (or whether we even want it to be added). It is not so rare to have multiple rounds of back and forth between the API-design side and the actual implementation work.
Which artists would it make sense to have a bbox for?
A more general reason why I raised this point is that the current implementation seems to be very tied down to QuiverKey implementation details, but a more generic one (based on get_window_extent) may be possible.
I have not followed all the details here, but currently QuiverKey does not have a get_window_extent method. I think that in order to implement one, you would need most of the logic currently in _draw_bbox. So, regardless of what the final implementation is, it might be a good idea to factor that out. This would have the added bonus of closing #18530.
Please understand that not all developers read all discussions at the same time nor do we all necessarily agree immediately on the best way to implement a feature (or whether we even want it to be added). It is not so rare to have multiple rounds of back and forth between the API-design side and the actual implementation work.
Yes, that makes sense, I'm still new at this.
So @story645, @rcomer, in summary, what do you want me to do? I understood your points and views, but I'm unsure what the final decision is.
I don't think anyone wants you to do anything. Again, the tricky part is figuring out what to do, not so much how to do it.
@story645, @rcomer, in summary, what do you want me to do?
Like @anntzer said, we've gotta come to consensus for quiverbox. I think for now, you can work on @rcomer 's suggestion of creating a get_window_extent method. That will close an outstanding issue, and if consensus ends up being to generalize the solution than having written the specific solution will help you write the general form.
Yes, I know, I was talking about the part of being more generic. Ok, thank you, @story645 .
I'm not really sure it's actually a good idea to add bbox support in each artist class one at a time. Instead, it may be more viable to have something like
FancyBboxPatch.around_artists(list-of-artists, **kwargs); or at least have bbox support in the base artist class, relying on the generic get_window_extent method to figure out how big to draw the bbox?
@anntzer I agree that a systematic approach to boxes around Artists would make sense. I have no clear picture yet which approach is the best. Some ideas - feedback welcome:
- Artist should not have a "builtin" box capability, in the sense that these are exposed via artist properties (something like
box_color,box_linewidth). This puts too much onto the base class. - Artists could own a box as a child artist. The box would then follow the artist dimensions. This could be convenient, but we should consider:
- would the box be included in the Artist extent? There are reason to do so and reasons to not
- This does not allow for boxes around multiple artists
- Alternatively, boxes could contain Artists in the sense of
FancyBboxPatch.around_artists(list-of-artists, **kwargs). Possible obstacles here are that either the boxes are drawn around after-the-fact, which would likely exclude them from layouts, or the Boxes are proper containers, which implies the artist tree is different in one case you have the direct artists (e.g. aTextinstance for the title), but if you draw a box around this, there's aFancyBoxPatchthat takes the logical position of the title and itself contains theText.
the boxes are drawn around after-the-fact, which would likely exclude them from layouts
Would this also restrict the box to only being a frame with no facecolor?
Would this also restrict the box to only being a frame with no facecolor?
No. We can set the zorder appropriately so that the box is drawn before the artist.