Thanks for fixing this. I can't think of any reason not to just have them as kwargs with defaults ofNone as you said in the bug report, but maybe I'm missing something.

@dstansby Yes, I'm still not sure. But I largely copied fromaxes.legend and its done with*args, *kwargs as well, so I did that here.

OK, I refactored. Nowfigure.legend andaxes.legend have exactly the same argument parsing (now inlegend._parse_legend_args).

Probably need the docs forfigure.legend changed. Its possible this is non-backwards compatible.figure.legend used to allow three positional arguments, with the third beingloc, but I don't thinkaxes.legend ever did.

Question: Is there a way forfigure.legend andaxes.legend to share part of their docs since the calls are the same now?

This is not passing a legend test that I don't understand so I'm having difficulty debugging...

deftest_legend_handler_map(self):lines=plt.plot(range(10),label='hello world')withmock.patch('matplotlib.axes.Axes.''get_legend_handles_labels')ashandles_labels:handles_labels.return_value=lines, ['hello world']plt.legend(handler_map={'1':2})handles_labels.assert_called_with({'1':2})

It seems that both instances of legend should have pass-through kwarg documentation.

It seems theright way to do this is for each keyword argument to have aset_arg that has anACCEPTS: line in the doc. But I'm not doing that. So I assume I can define a kw doc string and then reference it somehow.

Ah, I knew I'd seen it somwhere.colorbar_doc is a great example.

Well, that escalated quickly. I thought this was going to be a ~5 line fix....

@tacaswell We could easily go back to the 5-line fix...

But the fundamental problem was that*args, **kwargs were being handled differently betweenfigure.legend andaxes.legend for no good reason, and it will just be a maintenance headache in the future. Worse thelegend.Legend keywords were described three different places in three different formats, inconsistently in each place.

The changes are actually pretty small.

1. moved the argument-parsing logic out ofaxes.legend intolegend._parse_legend_args
2. used the same parsing infigure.legend
3. changed the three docstrings to refer tolegend_kw_doc.

I need to do some cleanup, and I still have no idea how to address the failingmock test (see comment above), but otherwise I hope this is an improvement.

Oh, hmmmm.fig.legend() did this funky thing where if you called

fig,axs=plt.subplots(2,1)l1=axs[0].plot(x,'r',label='boo')l2=axs[1].plot(x,'r',label='boo')fig.legend()

It would only put 'boo' in the legend once. This PR doesn't respect this. Easy to fix, but do we really want to?

I guess we "have to" for backcompatibility but that sounds like yet another thing I'd like to get rid of... (for example Axes.legend does not provide the same functionality).

@anntzer Ahem, now it does ;-) Well, if this PR gets accepted. I actually think there is some sense to it, but I wouldn't have offered the functionality if it'd just have been up to me...

I can't say I really hate this feature (so if others like it I'm fine with it), but in my view it's a bit similar to auto reuse of axes (#9037 and linked isses) and would prefer not having it for the same reason.

Modulo the handling of a positionalloc being inconsistent between axes and figure 👍

@tacaswell Can we just deprecate the third positional argument in both? Right now the code doesn't care if its called fromfigure oraxes.

That said, I'm not atall arguing for this third positional argument.

In the spirit of minimal change / semver I would rather deprecateloc onFigure.legend in 2.2 rather than 2.1.1. It is alreadyremoved fromAxes.legend from 1.5 so we don't need to deprecate it there ;)

@tacaswell Allowing the third positional argument is back inAxes.legend with this PR, becauseall the code to handle the arguments is the now same betweenAxes.legend andfigure.legend. Thats how this PR got rather large-looking. If we handlefigure andAxes differently, their code bases will diverge again, and thats how we ended up in this mess in the first place!

I'm suggesting we silently go back to allowing the third positional argument inAxes.legend, but deprecate if anyone happens to use it, and deprecate it infigure.legend as well. Then we can take it out again for 2.2. Its not like it hurts anything to handle the third positional argument.

A user will discover it and complain when it goes away. I the current behavior can be maintained on top of this refactor without too much extra complexity.

InAxes.legend()

deflegend(self,*args,**kwargs):handles,labels,extra_args=_parse_legend_args([self],*args,**kwargs)iflen(extra_args):raiseTypeError(....)l=legend.Legend(handles,labels,**kwargs)    ...

and inFigure.legend()

deflegend(self,*args,**kwargs):handles,labels,extra_args=_parse_legend_args(...)l=legend.Legend(handles,labels,*extra_args,**kwargs)    ...

and in_parse_legend_args

extra_args= ()if ...:    ...eliflen(args)>=2:handles,labels=args[:2]extra_args=args[2:]returnhandles,labels,extra_args

The current behavior ofFigure.legend is thatall inputs tolegend.Legend can be passed as positional so this is a bit worse than I initially thought.

Ah OK, I understand. Thats easy enough, if a bit of a PITA.

Not sure I understand your last comment, though. Currently onlyhandles,labels, andloc can be passed as positional, andloc actually gets converted to akwarg.

In 2.0 the truncatedFigure.legend is

deflegend(self,handles,labels,*args,**kwargs):l=Legend(self,handles,labels,*args,**kwargs)self.legends.append(l)l._remove_method=lambdah:self.legends.remove(h)self.stale=Truereturnl

so the API change from 2.0 -> 2.1 is bigger than just breakinghandlers andlabels as kwargs. If we are going to go back to the < 2.1 behavior we should bring it all back, add deprecation warnings to positional args in 2.2 and then remove in the next release.

Sorry, I was using confusing tense!

Believe it or not,ax.legend(arg1) canalso accept just a list of handles, a fact I found out only because@choldgraf did it inhttp://matplotlib.org/devdocs/gallery/text_labels_and_annotations/custom_legends.html#sphx-glr-gallery-text-labels-and-annotations-custom-legends-py

But, there is no way to attach a proper label to the legend when you call the legend like this, even if the underlying handles have labels! In master:

N=10data= [np.logspace(0,1,100)+np.random.randn(100)+iiforiiinrange(N)]data=np.array(data).Tcmap=plt.cm.coolwarmrcParams['axes.prop_cycle']=cycler(color=cmap(np.linspace(0,1,N)))fig,ax=plt.subplots()lines=[]foriinrange(N):lines+= [ax.plot(data[:,i],label='%s'%i)]print(lines[-1])ax.legend(lines)

That is broken beyond belief in my opinion, because:

fig,ax=plt.subplots()lines=[]foriinrange(N):lines+= [ax.plot(data[:,i],label='%s'%i)]print(lines[-1])ax.legend(lines[::3])

is a completely reasonable thing to do, but yields:

I won't fix in this PR, but wanted to register my dismay.

I thinkax.legend(arg) should accept either a list fo strings (old label format), or a list of handles. If the first, it does what it does now and just finds the first N handles to attach the strings to. If the latter, it provides a legend for that list.

The arg = list of handles is not documented, and I think@choldgraf example above should be modified because it is doing somethinglegend isn't really meant to do (all tests and docs passed for my change before this example was put into master). I think we should add support for lists of handles, but only display them if the label is explicitly set, and document this asnew behaviour. If you'd like that as a 2.2 separate PR, thats fine. I think the PR here, as it stands, gives us back what we had before 2.1. Pinging@tacaswell@anntzer@dstansby Thanks!

As your second example shows,ax.legend is not "accepting a list of handles" (you will note that the artists that went into the legend in the second example are not every third, but the first 4); instead, it is interpreting the (positional first) argument as a list of labels (after forcefully converting them usingstr) and assigning them as labels to the firstn artists.

Yes this is nonsensical, but should really be the object of a separate PR.

This is ready to go, unless we find somethingelse whacky that the various versions of legend do...

I have two approvals, but I assume its bad form to self-merge?

@jklymak I think we did something funny with git :(

Sorry! My fault. Let me re-push. I made a local change and was pushing onto your change. But I got your change. I won't touch it until tonight, honest!

@dopplershift Yes fair enough - its easy to fix the docs later. I'll add a note that this is one place to fix in#9414 if its not there already....

Ummm, help! The original example no longer works!!

fig,ax=plt.subplots(1,2)hs= []lab= ['Boo','Hoo']fornn,axxinenumerate(ax):h,=axx.plot(np.arange(10))hs+= [h]fig.legend(hs,lab)plt.show()

/Users/jklymak/matplotlib/lib/matplotlib/legend.py:932: UserWarning: Legend does not support [<matplotlib.lines.Line2D object at 0x107cfc3c8>] instances.A proxy artist may be used instead.See: http://matplotlib.org/users/legend_guide.html#creating-artists-specifically-for-adding-to-the-legend-aka-proxy-artists  "aka-proxy-artists".format(orig_handle)/Users/jklymak/matplotlib/lib/matplotlib/legend.py:932: UserWarning: Legend does not support [<matplotlib.lines.Line2D object at 0x107cfc908>] instances.A proxy artist may be used instead.See: http://matplotlib.org/users/legend_guide.html#creating-artists-specifically-for-adding-to-the-legend-aka-proxy-artists  "aka-proxy-artists".format(orig_handle)

Ooops, false alarm. I had a typo in my test!

importnumpyasnpimportmatplotlib.pyplotaspltfig,ax=plt.subplots(1,2)hs= []foraxxinax:h,=axx.plot(np.arange(10))hs+= [h]fig.legend(handles=hs,labels=['Boo','Hoo'])plt.show()

works fine!