Add pie artist - #32359
Add pie artist#32359robertoffmoura wants to merge 15 commits into
Conversation
|
Thanks a lot for working on this. I think this PR is going in a very promising direction, in particular by making Pie a proper composite object and moving ownership of the wedges/texts under it rather than keeping them as unrelated direct children of the Axes. Though we have to carefully decide whether we can afford the related API breakage and whether we could smoothen the transition. On a broader context, I think we need to think quite carefully about the broader object model this implies. One question is property propagation. Once Pie is an Artist that owns other Artists, what should e.g. set_visible, set_zorder, set_alpha, transforms, clipping, picking, etc. mean for the aggregate and its children? Some of these have fairly natural group semantics, while others do not. I would prefer not to make these decisions pie-by-pie if what we are really introducing is a general notion of a compound/grouped Artist. Relatedly, I wonder whether this points towards a broader ArtistGroup / CompoundArtist abstraction. Potentially there is a subset of the current Artist functionality that is structural—Axes/Figure association, ownership, child traversal, lifecycle, staleness, drawing order, visibility, removal, etc.—and could eventually live in a common base of both ordinary Artists and grouped Artists. A group would then not automatically inherit visual properties for which propagation semantics are unclear. The proposed get_legend_handles() mechanism also looks useful, but I think we should validate that abstraction against the other existing aggregate plot types before making it public API: bars, errorbars, stems, and stackplots in particular have different 1-vs-N legend-entry semantics. So overall: I think this is very much the right direction, and it gives us a concrete implementation to reason about. But because it potentially establishes a more general compound-Artist model, I would like us to spend a bit more time on those semantics before locking in the API. |
I think also the interaction/boundary between data container and artist container as data containers get phased in. Attn @ksunden |
This PR is a follow up to #32320. It adds a Pie artist class and deprecates PieContainer.
legend.pychecks for aget_legend_handlesmethod, so compound artists can expose their handles.Previous state:
State after this PR:
AI Disclosure
AI used to gain understanding of the existing code