Skip to content

Conversation

@rcomer
Copy link
Member

@rcomer rcomer commented Nov 17, 2024

PR summary

The proposal evolved through the below discussion. The idea is to introduce a new wedge_labels parameter to pie which supersedes autopct, via its use of the str.format method, but it can also take a list of labels consistent with the existing labels parameter.

  • Introduce wedge_labels as a shortcut to the pie_label method introduced in ENH: introduce PieContainer and pie_label method #30733.
  • Introduce wedge_label_distance (default 0.6) to specify how far the wedge_labels are from the pie centre.
  • Deprecate the labeldistance parameter in two steps: First change the default to None (which means labels are only used for the legend). Later remove this parameter altogether - direct to use wedge_labels instead.
  • Discourage using autopct in favour of wedge_labels.

PR checklist

@rcomer
Copy link
Member Author

rcomer commented Nov 17, 2024

@rcomer rcomer force-pushed the pie-fmt branch 2 times, most recently from e5db08a to 925704e Compare November 17, 2024 16:42
@rcomer rcomer added this to the v3.11.0 milestone Nov 17, 2024
@timhoffm
Copy link
Member

If we are touching the existing API, should we reconsider more fundamentally?

Currently, pie() has two sets of annotations

  1. "label", which are strings and by default displayed outside next to the wedges
  2. "pct", which are percent numbers and by default displayed inside the wedges

Some observations:

  • "pct" is a too specific name, "numbers" would be better.
  • That labels are outside and pct inside is an arbitrary choice.

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 inner_label and outer_label, and each could be (i) an explicit list of strings (ii) a function (iii) a format-string, likely {}-based, because that allows direct percent-formatting without needing numeric action on our side. However, I belive this would be too much of a change for the current API.

Therefore, I propose to

  • keep labels, label_distance as is
  • introduce inner_labels and inner_label_distance. inner_labels should take (i) a list (ii) a function (iii) a {} format string
  • autopct translates to inner_labels and giving both is an error. Discourage autopct in favor of inner_labels and describe the transition (note that autopct's %-format and callable take values that are scaled by 100).
  • pctdistance translates to inner_label_distance and giving both is an error. Discourage pctdistance in favor of inner_labels_distance and state that you should use the variable that is matching to the one used to define the labels inner_labels=..., inner_label_distance=... and autopct=..., pctdistance=....
  • optionally, expand labels to also take (ii) a function (iii) a {} format string

Does that sound reasonable?

@rcomer
Copy link
Member Author

rcomer commented Nov 17, 2024

I am not sure about inner_labels because there is nothing to stop you swapping the positions of these with labels (using the distance parameters), at which point they become outer labels. We also need some way to choose what is being passed through the function or format string: existing functionality is percentages but the feature request is to support the absolute values.

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 🤔

@rcomer
Copy link
Member Author

rcomer commented Nov 18, 2024

Also note that labels is special because it will be automatically used by legend (and if you want them in the legend and not on the pie you set labeldistance=None). So that's another reason not to touch that. So the other labels could be named something relative to that: extra_labels, secondary_labels....?

The original proposal in #19338 was for wedgelabels, so just throwing that in...

@rcomer rcomer added the status: needs comment/discussion needs consensus on next step label Nov 18, 2024
@timhoffm
Copy link
Member

What I've missed in my above thought is that {} direct percent-formatting takes case of the factor 100 between relative and percent numbers, but it does not solve the issue where to pass relative (or percent) vs. absolute numbers to the formatter. That extra information that we'd need to get in somehow.

I'm trying to find a consistent way to map the option space:

  • We need potentially two labels per wedge, typically one of them is inside and one is outside, but the positions are freely adjustable.
  • We have three kinds of information: 1. Fixed arbitray labels 2. absolute numbers 3. percent (or relative numbers)
    Note that the number variants are convenience. A user could take the input values, transform them to whatever string they want and pass them as fixed label.

There are two ways for structuring:

  • Have two label mechanisms that are structurally equivalent (i.e. allow fixed values and numerics). The distinction would be the (default) position. This was my thought above.
  • Distinguish by semantics, i.e. one set of args for fixed labels, one set of args for numeric labels, ...
    Here we actually get into some complication because either we have to define the sets of args (fixed, absolute, percent) but map them to two positions; or we have to merge absolute and percent to one parameter to one numbers parameter and add a flag to determine absolute vs relative. Either way is messy.

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 inner_labels and outer_labels (or labels when considering compatibility) as lists of fixed texts. Everything else is convenience. Possibly, we don't even need absolute value support then. It is just pie(x, inner_labels=[f'{xi:.1f}' for xi in x]) IMHO this is even better - because more explicit - than pie(x, absolutefmt="%.1f"). Percent formatting is only marginally more complex pie(x, inner_labels=[[f'{xi:.1%}' for xi in x/x.sum()]).

I therefore revise my above proposal to the following minimal change:

  • keep labels, label_distance as is
  • introduce inner_labels and inner_label_distance. inner_labels should take a list only
  • autopct translates to inner_labels and giving both is an error. Describe autopct as a higher-level convenience function to get percent values into inner_labels.
  • Document pctdistance as equivalent to inner_label_distance and giving both is an error. State that you should use the variable that is matching to the one used to define the labels inner_labels=..., inner_label_distance=... and autopct=..., pctdistance=....

One can later decide whether both of the labels parameters should grow some formatting / function capability.

@timhoffm
Copy link
Member

timhoffm commented Nov 18, 2024

I've just read your comment on legend. That's actually making it more complicated.

plt.pie(x, labels=['a', 'b', 'c'])
plt.legend()

plots both the outer labels and the legend, which is most likely not what one wants. One has to additionally pass labeldistance=None, but that prevents using the outer labels for something else (e.g. absolute numbers).

Note that outer labels are a good option if wedges become tiny. Therefore I fundamenally would want capability to put anything outside, e.g.
image

I believe right now, the only way to add a legend there is explicitly passing the artists and legend labels to legend().

A green-field solution would be to have labels only for the legend and explicit outer_labels and inner_labels on top. This separates concerns. However that'd be quite disruptive.

I have to rethink what we can do here.


Edit: I think we could make a clean transition path for separating labels into labels (legend only) + outer_labels:

  • If as user uses labels with a not-None labeldistance, they want an outer-wedge label. Add a deprecation here and state that they should use outer_labels (and outer_label_distance if needed). Add an additional note that they need to pass the same list to labels as well if they want to keep legend entries.
  • If a user has used labels=..., labeldistance=None they only want and get a legend. Nothing to do for a start. One can deprecate and remove the labeldistance parameter, when the above deprecation on for labels generating outer texts has run out. Then labels will always only create legend entries and the labeldistance parameter has no effect anymore.

The only downside is the need for users to adapt their code. But that's inevitable if we want a logical separation between legend labels and wedge labels.

This all is independent of the "absolute value" topic, but would fit together with the proposed solution introducing inner_labels there.


I am not sure about inner_labels because there is nothing to stop you swapping the positions of these with labels (using the distance parameters)

I'm not concerned about this. Practically, there's no need to swap - you'll rather swap the lists passed to labels/inner_labels. But if you do that's a very conscious and explicit decision.

naming extra_labels, secondary_labels, wedge_labels

  • wedge_labels is a bit ambiguous. While the inner labels are on the wedge, both labels are per-wedge. The outside labels in the plot above could also be regarded as "wedge_labels".
  • extra_labels, secondary_labels implies a hierarchy, which IMHO does not exist. Both places are logically equivalent. Having extra_labels/secondary_labels without labels would be a valid use case, but sounds a bit awkward. Additionally, extra/secondary does not give you an idea what they are used for / where they will appear.

@rcomer
Copy link
Member Author

rcomer commented Nov 18, 2024

I am also wondering if it’s worth factoring out a public pie_label method, to rhyme with bar_label. The existing labels and autopct could remain as conveniences that internally call this method, but a user could also call it directly as many times as they like to get whatever combination of labels they like.

@timhoffm
Copy link
Member

pie_label method: I'm a bit sceptical. If we didn't have labelling functionality at all in pie, that might be the way to go. But as is, there's not much new functionality to be introduced via a pie_label. More than two positions (inside/outside) would be a very edge case. And the fancy number formatting/percent stuff is quite optional, because you can do it out-of-the-box with little overhead I wouldn't introduce new just API for that.

What may be a good idea is a private _wedge_text() method as internal refactoring, which would simplify the label handling in pie.

@rcomer
Copy link
Member Author

rcomer commented Nov 18, 2024

OK let me see if I can summarise the current thinking:

  • Introduce inner_labels and outer_labels which take a list of strings or a format string to be formatted with .format(abs=abs, frac=frac).
  • Introduce inner_label_distance (default 0.6) and outer_label_distance (default 1.1) to specify how far the above are from the pie centre.
  • Deprecate using labels with not-None labeldistance - direct to use outer labels instead.1
  • Discourage2 using autopct in favour of inner_labels. Passing both is an error.
  • Passing pctdistance and inner_labels is an error.
  • Passing inner_label_distance and autopct is an error.

Footnotes

  1. when this deprecation has expired, deprecate the labeldistance parameter altogether.

  2. it might be nice to eventually deprecate autopct as having 4 parameters for labels is confusing.

@timhoffm
Copy link
Member

timhoffm commented Nov 18, 2024

Good summary! This is one viable way to make the API more consistent.

I think we should offer the formatting option as it will be hard to discourage use of autopct if the replacement is less convenient.

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 pie(x, inner_labels="%.1f").

The function approach is a bit better, because you could support two signatures def fmt(abs_value: float) -> str and def fmt(abs_value: float, total: float) -> str. The first one is a simpler version for convenince, the second one would support percents as pie(values, inner_label=lambda x, total: f'{x/total:.1%'}).


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 textprops are shared and rotatelabels only applies to the outside.

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 pie(). We'd likely need to create a semantic artist PieWedge or a kind of Pie container class to handle this.
Problem 2: Removing all labelling capability from pie and forcing the use of additional functions feels a bit drastic. The alternatives would be to keep some labelling capability and that's not great either: (i) Keeping labels as the simple generic variant keeps the entanglement with legend (ii) Keeping autopct is very specific (iii) changing to a new single labels API forces a migration and makes keeping an additional text labels API only half as valuable.

If feel this is less good than the summarized approach.

Footnotes

  1. Edit: It's a bit unusual, but we might solve the absolute/relative issue with a text prefix inner_labels="percent:%.1f" could replace autopct="%.1f" - or inner_labels="fraction:{:.1%} when using format-string notation.

@jklymak
Copy link
Member

jklymak commented Nov 18, 2024

Completely green fielding this issue a I'm pretty ignorant about pie - I would personally get rid of autopcnt - it's not like our users cannot calculate the data in terms of a percentage, and the pie chart would look the same. Then the formatting of the quantitative label would be straightforward.

@rcomer
Copy link
Member Author

rcomer commented Nov 19, 2024

I would prioritise formatting fractions over absolute values:

  • Since plotting fractions is basically the point of pie I don’t think it’s too unintuitive that those would have the convenience in terms of labelling. The canonical example would be pie(x, inner_labels="{:.1%}"). Also since pie already calculates the fractions in order to draw the wedges it feels a bit annoying/redundant for the user to have to pre-process the data just for the labels (even if it is a simple and cheap calculation).
  • Absolute values are slightly more straightforward to convert to the desired string. In fact if the user has ints then they may not care about formatting, so they could just pass the list of numbers as labels and the Text constructor will take care of converting them to strings.

Edit: I guess you could detect whether the format string contains "%}". If it does, give it fractions. If not give it absolute values.

@timhoffm
Copy link
Member

timhoffm commented Nov 19, 2024

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 {.1f} and {.1%} relate to each other. We shouldn't mess with that. Then let's rather do the "percent:..." or "relative:..." or "fraction:..." prefix to the format string.

@rcomer
Copy link
Member Author

rcomer commented Nov 19, 2024

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%)'

@timhoffm
Copy link
Member

Named placeholders would give a lot more flexibility...

Great idea. Add this format-string as acceptable input for inner_labels and outer_labels in #29152 (comment) and we have a compact and clean API.

The fundamental question is: Do we want to go through the hassle of repurposing labels to only apply to legend. If yes, this is the reasonable way forward. If no, only do inner_labels as a replacement of autopct. I'm 0.5 on going all the way (but slowly, i.e. start with a pending deprecation).

@rcomer
Copy link
Member Author

rcomer commented Nov 20, 2024

I am in favour of making labels only for legend.

  • API consistency: almost everywhere that you pass label(s) in Matplotlib, you mean you want them in the legend.
  • For the case where you want both a legend and numbers outside the wedges, using labels=list_of_strings, outer_labels=... is more intuitive to me (and simpler) than labels=list_of_strings, labeldistance=None, inner_labels=..., inner_label_distance=1.1.

@timhoffm
Copy link
Member

timhoffm commented Nov 20, 2024

Side note on naming: I've briefly pondered whether in would make sense to bikeshed outer_labels, inner_labels to replace the "label" part; motivation: set it more clearly apart from labels; e.g. go for wedge_text or similar. However, we have ample precedence for *_label usages independent of labels mapping to legend: bar_label, clabel (contour label), tick_labels, xlabel, ylabel. Therefore outer_labels and inner_labels seem perfectly fine.

Edit: The only other alternative that may be worth considering is to make a single wedge_labels, wedge_label_distance that accepts a single list or a tuple of lists so that one can do:

plt.pie(x, wedge_labels=['a', 'b', 'c'], wedge_label_distance=0.5)
plt.pie(x, wedge_labels=(['a', 'b', 'c'], ['A', 'B', 'C']), wedge_label_distance=(0.5, 1.2))
plt.pie(x, wedge_labels="{frac:.1%}")
plt.pie(x, wedge_labels=("{frac:.1%}", "{abs:.2f}"), wedge_label_distance=(0.5, 1.2))

instead of

plt.pie(x, inner_labels=['a', 'b', 'c'], inner_label_distance=0.5)
plt.pie(x, inner_labels=['a', 'b', 'c'], inner_label_distance=0.5, outer_labels=['A', 'B', 'C'], outer_label_distance=1.2)
plt.pie(x, inner_labels="{frac:.1%}")
plt.pie(x, inner_labels="{frac:.1%}", inner_label_distance=0.5, outer_labels="{abs:.2f}", outer_label_distance=1.2)

@story645
Copy link
Member

story645 commented Nov 20, 2024

The only other alternative that may be worth considering is to make a single wedge_labels, wedge_label_distance that accepts a single list or a tuple of lists

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 pie_label? that could then take a third parameter "fmt" that accepts a list of formatting strings/functions/Formatter Objects

@timhoffm
Copy link
Member

Though could this also be the external function pie_label?

Not easily. See #29152 (comment) for the issues I see with pie_label function.

@rcomer
Copy link
Member Author

rcomer commented Nov 20, 2024

What would be the default for wedge_label_distance?

@story645
Copy link
Member

As a follow up, could pie_label expand to support all sort of styling for the text? As a follow up issue?

@timhoffm
Copy link
Member

timhoffm commented Oct 22, 2025

Yes pie_label() will be allowed to go fancy.

Re: Design - I believe we need some container as the return type of pie().

We currently have

patches, texts = pie()
patches, texts, autotexts = pie(..., autpct=...)

We should end up with

pie : PieContainer = pie()

so that the above stays valid, i.e. it supports unpacking patches, text = pie (at least for a transition phase). We also want a high-level signature pie_label(pie: PieContainer) and not pie_label(pie: list[PieWedge]).

I'm slightly unclear whether we want a pure container in the sense of Container or a semantic artist like StepPatch. The difference being that Containers basically just bundle some artists, but semantic artists, add special logic and meaning. Maybe it's also both here.

@jklymak
Copy link
Member

jklymak commented Oct 22, 2025

Why wouldn't you return a list of PieWedges? Do the wedges need to know each other for some of the formatting?

The point being you could just label some of the wedges if you just pass a sublist of wedges.

@timhoffm
Copy link
Member

Because when regarding Pie as a semantic entity, you can e.g. add a pie.set_data() method. You can't do that for a list of wedges. Note: This is exactly why we introduced StepPatch for stairs() and did not use a generic PathPatch.

@jklymak
Copy link
Member

jklymak commented Oct 22, 2025

Ok that's fair enough. I guess the wedges are not independent in that if you change one you need to change the others.

@rcomer
Copy link
Member Author

rcomer commented Oct 23, 2025

OK, but can we say the initial implementation of PieContainer has immutable data? I am starting to worry about the size/scope of this...

@timhoffm
Copy link
Member

timhoffm commented Oct 23, 2025

Sure. I think this would be more or less sufficient for a start.

class PieContainer:
    def __init__(patches, texts, autotexts=None):
        self._patches = patches
        self._texts = texts
        self._autotexts = autotexts

    def __iter__(self):
        # needed to support unpacking into a tuple for backward compatibility
        return (
            (self._patches, self._texts)
            if self._autotexts is None
            else (self._patches, self._texts, self._autotexts)
        )

One could add more methods to increase backward-compatibility with tuples, but I suspect basically all use cases will immediately unpack (patches, texts = ax.pie(....)).

You don't even have to make the data public - and I wouldn't right now. It's likely that we'll want to store multiple sets of labels, and then texts : list[Text] is likely not what we want to have as a long-term API.

@rcomer
Copy link
Member Author

rcomer commented Oct 25, 2025

There seems to be a consensus for the pie_label method. I will add that in a new PR and this one will be "PR2".

@rcomer rcomer marked this pull request as draft October 25, 2025 08:07
@rcomer rcomer changed the title ENH: introduce wedge_labels parameter for pie ENH: introduce *wedge_labels* parameter for pie Nov 29, 2025
@rcomer rcomer changed the title ENH: introduce *wedge_labels* parameter for pie ENH: introduce wedge_labels parameter for pie Nov 29, 2025
@rcomer
Copy link
Member Author

rcomer commented Nov 29, 2025

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 pie code.

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 pie_label method.

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 pie_label. In some cases I simply removed one or both sets of labels since they are not needed for what a particular example is showing. E.g. they are not needed to show the effect of passing colors or hatch; to demonstrate explode or radius one set is useful to show that the text positions follow the size/positions of the wedges.

@rcomer rcomer marked this pull request as ready for review November 29, 2025 12:17
@rcomer rcomer added the DO NOT MERGE Last-resort; prefer specific mechanisms if possible: draft PR, request changes, specific labels label Nov 29, 2025
fracs = x
if labels is None:
labels = [''] * len(x)
elif labeldistance is False:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a benefit in introducing False. AFAICS

Suggested change
elif labeldistance is False:
elif labeldistance == 1.1:

would have the same effect. The only difference is that False allows to suppress the deprecation warning by setting labeldistance=1.1 explicitly. But that only buys you some time as the parameter will be removed eventually. You could either ignore the warning, or switch to the newer methods, or if needed version-gate your code.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Buying time is part of the point, since I think we agreed somewhere that functionality should not be deprecated until its alternative has been available for at least one meso version. Though now I look, that discussion did not make it into the Rules. I note that at #29152 (comment) you were keen to do this slowly.

Also it would be strange to me that passing 1.1 gives me a warning when any other number does not.

Comment on lines +3758 to +3777
if wedge_labels is not None:
self.pie_label(pc, wedge_labels, distance=wedge_label_distance,
textprops=textprops)

elif labeldistance is None:
# Insert an empty list of texts for backwards compatibility of the
# return value.
pc.add_texts([])
else:

if labeldistance is not None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to prohibit wedge_labels is not None and labels is not None and labeldistance is not None. This would add two sets of labels. Currently, and after deprecation, we'll only add one set of labels.

Since wedge_labels is new, anybody who sets that could also adapt to labeldistance=None; and if needed use pie_label to add the second set of labels.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, good point.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API: changes DO NOT MERGE Last-resort; prefer specific mechanisms if possible: draft PR, request changes, specific labels Documentation: examples files in galleries/examples New feature status: needs rebase topic: plotting methods topic: pyplot API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants