I'm having difficulties getting the image comparison test to pass (none of them pass on my machine), so I'd appreciate if SO could regen the image and push on the branch for me.

It should be noted that None is in general used in the sense of "the default" in matplotlib, e.g. facecolor=None does not mean "no facecolor", but rather "use default facecolor". In most cases, "None" instead gives "no such thing".
For API consistency one might stick to that concept here as well.

A general question here: Can you explain a bit the envisionned difference between

pie(...., labels=None)

and

pie(..., labels=[....], labeldistance=None)

which is proposed here?

Sure...

import matplotlib.pyplot as plt

labels = 'ABC'
sizes = [10, 10, 10]

fig, (ax1,ax2) = plt.subplots(1, 2)
fig.set_figwidth(12)

# hard to do without this PR.
ax1.pie(sizes, labels=labels, startangle=0, labeldistance=None, frame=True)
ax1.axis('equal') 
ax1.legend()

ax2.pie(sizes, labels=None, startangle=0, frame=True)
ax2.axis('equal') 
ax2.legend()

Outputs:

Stderr (for second graph): No handles with labels found to put in legend.

AS I said in the PR description, you need to pass labels for legend() to work.

I'm happy to change the api. None is one of the only "obvious" candidate, with 0, or a sentinel string if we don't want to complicate the API. The other possibility is an extra keyword.

While not a fan of extra kwargs I think that’s preferable here. I’d never think to read the labelspacing description far enough to know that’s how you control whether the label is shown or not, so a new kwarg is more discoverable in my opinion.

Ok, thanks. So given

import matplotlib.pyplot as plt
labels = 'ABC'
sizes = [10, 10, 10]

previously you could do

wedges, texts = plt.pie(sizes)
plt.legend(wedges, list(labels))

or (but that's hacky and might cause problems with layout managers or the tight savefig(?))

plt.pie(sizes, labels=labels, labeldistance=-100)
plt.legend()

With this PR:

plt.pie(sizes, labels=labels, labeldistance="None")
plt.legend()

All codes produce the same desired output.
I'm personally not convinced that this is necessary, but given there is some demand for it, why not.

However introducing a new argument just for this purpose feels like a bit of overkill. I would argue that either you read the complete documentation or you need to stick with the current possible solution, no?

Another annoyance is plt.pie(sizes, ...) sometime returns a 2-tuple sometime a 3 tuples depending on the value of the autoprct kwargs.

I would argue that either you read the complete documentation or you need to stick with the current possible solution, no?

With that reasoning we would never add features right ? :-)
Creating a legend manually is often painful, and seem to kind-of be against the usual ways of matplot lit to let user pass label(s)=... and then just call "legend()". I'm ok using the hackish way, and it seem sufficiently easy to do that I sent a PRs, but I don't want to give you extra maintenance if you think it's unnecessary.

What I wanted to say is that the proposal with labelspacing="None" is fine with me. The "feature" this introduces is just not big enough to merit its own argument (like showlabels=False or so) in my eyes. But @jklymak has the opposite opinion, so let's wait for a third person to say something about it.

Concerning tests, would it maybe be possible to test this without image comparisson? Essentially you want to test if

• the returned texts is an empty list (len(texts) == 0)
• no labels are created and added to the axes (len(ax.texts) == 0)
• the legend still contains the labels,

  for lab, legtxt in zip(list(labels), legend.texts):
      assert legtxt.get_text() == lab
  

What I wanted to say is that the proposal with labelspacing="None" is fine with me. The "feature" this introduces is just not big enough to merit its own argument (like showlabels=False or so) in my eyes. But @jklymak has the opposite opinion, so let's wait for a third person to say something about it.

Sure.

Concerning tests, would it maybe be possible to test this without image comparisson? Essentially you want to test if

Sounds good to me one the desired API has been agreed upon.

labelspacing="None"

Also possible negative values as positioning with negative value does not make much sens, up to you.

I agree with @jklymak on that one.
@Carreau I can also help with the test image generation.

So I wasn't strongly agains the API here - lets merge as is after the images are fixed.

I'll try and do the images now...

I'm also happy to work on that next saturday at JupyterCon sprint if anybody is around.

OK, think I did that right - I pushed a working image to your directory.

To set up my deve environment I do:

git clean -xdf   # clean out the caches
cp setup.cfg.template setup.cfg   
edit setup.cfg   # turn on local freestyle and tests..
pip install -e .

If I don't do the clean than the install doesn't do the local freetype and tests fail.

... ok, and I think I fixed the flake8 complaint...

I tend to go for a separate showlabels argument as neither would I expect to look for deactivating the labels in labelspacing. Nor is labelspacing="None" in any way self-explanatory.

However, I wouldn't insist on the separate argument if a majority is fine with adapting labelspacing. Further remark: Have you considered labelspacing="nolabels"? While it's a bit less standard than "None" it would make the intent more clear in the present context where the name of the existing argument doesn't fit well with showing/hiding of the labels.

Thanks everyone!