ENH: introduce wedge_labels parameter for pie#29152
Conversation
e5db08a to
925704e
Compare
|
If we are touching the existing API, should we reconsider more fundamentally? Currently,
Some observations:
What we fundamentally need to keep:The two sets of annotations must stay. We'd loose functionality if we allowed only one label. Also, a generalization to more than two sets of annotations does not make sense. Side-note: If I was designing this from scratch, I'd likely focus on location Therefore, I propose to
Does that sound reasonable? |
|
I am not sure about Edit: though I guess by supporting the list of strings we allow the users to format those absolute values however they like within a list comprehension before passing them in 🤔 |
|
Also note that The original proposal in #19338 was for |
|
What I've missed in my above thought is that I'm trying to find a consistent way to map the option space:
There are two ways for structuring:
IMHO the "absolute values" request comes from the fact that the inner label is too specific in that it only handles percent. There's no API to get arbitrary text in there. A minimal green-field approach would be I therefore revise my above proposal to the following minimal change:
One can later decide whether both of the labels parameters should grow some formatting / function capability. |
|
I am also wondering if it’s worth factoring out a public |
|
What may be a good idea is a private |
|
OK let me see if I can summarise the current thinking:
Footnotes |
|
Good summary! This is one viable way to make the API more consistent.
formatting is a bit tricky because you have to decide whether the inputs should be absolute values or fractions. You cannot have both (at least not without an extra parameter 1). I'm inclined to expect absolute values by default, and that would still not give a replacement for autopct. We could decide to go with relative values, but it feels a bit unintuitive The function approach is a bit better, because you could support two signatures Alternative: pie_label()I've thought about this a bit more. The idea is fundamentally reasonable. On the plus side, we get better decoupling and full control over each label set - currently Problem 1: AFAICS, we cannot retrieve the underlying data value from a wedge. Therefore no external function can create labels based on the output of If feel this is less good than the summarized approach. Footnotes
|
|
Completely green fielding this issue a I'm pretty ignorant about |
|
I would prioritise formatting fractions over absolute values:
Edit: I guess you could detect whether the format string contains "%}". If it does, give it fractions. If not give it absolute values. |
Yes, I've briefly thought about that too, but it's too much magic 🪄 . There's a standard expectation how |
|
Named placeholders would give a lot more flexibility... In [2]: '{abs:d}'.format(abs=42, frac=0.8)
Out[2]: '42'
In [3]: '{frac:.1%}'.format(abs=42, frac=0.8)
Out[3]: '80.0%'
In [4]: '{abs:d} ({frac:.0%})'.format(abs=42, frac=0.8)
Out[4]: '42 (80%)' |
Great idea. Add this format-string as acceptable input for The fundamental question is: Do we want to go through the hassle of repurposing |
|
I am in favour of making
|
|
Side note on naming: I've briefly pondered whether in would make sense to bikeshed Edit: The only other alternative that may be worth considering is to make a single instead of |
I really like this proposal b/c it removes the semantic issue of being able to position inner/outer label anywhere, and it gives people the flexibility to have as many sets of labels as they want. Though could this also be the external function |
Not easily. See #29152 (comment) for the issues I see with |
|
What would be the default for |
piepie
piepie
|
This one is now ready for review again, but should not be merged before the v3.11 branch is created. I am happy that this is much cleaner than the previous iteration, which was keeping track of too many things within the Unlike the previous iteration, I have not introduced a rotate_wedge_labels parameter. I think rotation is not a particularly common case, and anyone who does want it can use the I have removed all uses of the labels and autopct parameters from the examples in favour of either wedge_labels or a separate call to |
|
3.11 is branched; I don't know if you have followup corrections to make or just a rebase. |
|
I have rebased and also changed the default labeldistance from |
|
I think I addressed all comments here. Please shout if I missed something. |
|
Thanks @rcomer for seeing this through! |
|
Thanks for all the discussion and reviews! |

PR summary
The proposal evolved through the below discussion. The idea is to introduce a new wedge_labels parameter to
piewhich supersedes autopct, via its use of thestr.formatmethod, but it can also take a list of labels consistent with the existing labels parameter.pie_labelmethod introduced in ENH: introduce PieContainer and pie_label method #30733.None(which means labels are only used for the legend). Later remove this parameter altogether - direct to use wedge_labels instead.pie#29152 (comment).autopctin favour ofwedge_labels.PR checklist