I like the idea of a generalized way to do legends for PathCollections & PatchCollections, but wonder if this is overly complicated:

# produce a legend with the unique colors from the scatterax.legend(*scatter.legend_items(),loc=3,title="Classes")# produce a legend with a cross section of sizes from the scatterhandles,labels=scatter.legend_items(mode="sizes",alpha=0.6)ax.legend(handles,labels,loc=1,title="Sizes")

overlapping with the discussion in ##7383, I wonder if there shouldn't just be a subclass of legend that handles Path/Patch and cmap/norm stuff.

How is a single line "too complicated"?

If I understand correctly, a Legend subclass would then need to take the artist (in this case the PathCollection) as input, as well as those arguments needed to steer the output. In that sense you could replaceax.legend(*scatter.legend_items(**kw)) byax.enhanced_legend(scatter, **kw). So essentiallyenhanced_legend would take all the currently available 26 keyword arguments, and in addition those to manage what to show in the legend.

I also don't see much overlap to#7383. I think what's discussed there is interesting if at some point you want to allow for something likescatter(x,y, c=["A", "B", "B", "C", "A"])?

What is proposed in this PR is completely analogue to the existingContourSet.legend_elements() which creates handles and labels for contours. (... which makes me realize that the namelegend_items could of course be changed tolegend_elements to be consistent between those.)

How is a single line "too complicated"?

Sorry, I was reacting to the * notation and should have looked at this way more carefully. It's fine as is, especially if you go withlegend_elements() to make it consistent with existing functionality.

Actually ax.get_legend_handles_labels already exists. As does the concept of legend handlers. Any way this can be turned into a legend handler?

ax.get_legend_handles_labels exists indeed. What it does is to return the legend handles and labels that would produce a legend for an axesax. Example,

ax.scatter(...., label="mylabel")h, l = ax.get_legend_handles_labels()ax.legend(h, l)

would produce a single legend row with one scatter dot and the label "mylabel". It is hence equalvalent to simply callinglegend without arguments,

ax.scatter(...., label="mylabel")ax.legend()

Now what is possible using a legend handler would be to change the single handle from a single point to something more characteristic of a scatter, e.g.:

However, this is still a single legend entry for the complete scatter.

What this PR proposes is a way to getseveral legend entries forone single scatter plot. This is not possible with aHandler.

One might consider putting the proposed API forscatter.legend_elements(**API) intoax.get_legend_handles_labels(**API). I think this might be undesired because then the axes method takes all those arguments, which only make sense in the context of a scatter. At some point internally it would in any case still need to call something likescatter._legend_elements(**API). So I guess the proposed method here would still be needed and can be public as well.

The issue here is that the "one artist : one dataset : one legend entry" mapping that exist for exLine2D is badly broken byscatter.

A concern with the way we currently do legends is that the handlers are created by lookup up state held in theLegend class rather than asking theArtist objects in the draw tree what they think their legend handlers should be. On one hand, this is pretty nice as now the user can customize per-legend (so ex changing out the handler for everyLine2D object can be done in userspace at runtime), but can be a bit indirect and cumbersome.

How far can the notion of 'legend_elements' be generalized? Even if we only have a real implemenation for PathCollection, having aNotImpletened marker farther up the class tree would be good (so the next time we want something like this on another class we don't invent a different name!). Does it make sense to push this all the way up toArtist? I can see a use for that ingrave (https://github.com/networkx/grave/blob/master/grave/grave.py#L221) where the network artist may have strong opinions about what should go in the legend.

I think for now coming up with a consistent name and API forArtists to return a list of labels and handles as an optional public method is a good start and we can sort out how to integrate it into the auto-legend code later.

I think neither of
or
may actually be undesired in the general case. The initial proposal here was to make it easier to provide more handles and labels to the legend for a case where more detailed legend is desired.

What this shows is that it is probably rather hard to let the artist decide for a way of creating a legend by itself. It still needs some guidance by the user. This makes things rather complicated as in that case the arguments that steer the creation of one/several legend entries need to be given tolegend as well, adding up to the existing 26legend arguments and leading to a lot of useless arguments in cases where the respective artists is not even present in an axes. It also would be hard to distinguish which argument would need to be used for which artist. Alternatively, one could provide those arguments to the artist itself. So the artist has those properties stored and would use them if the legend calls itslegend_elements() method.

From a user's perspective, it may not actually make that much of a difference, whether to create the legend elements first and supply them to the legend (as in this proposal)

sc = ax.scatter(x,y,c=.., s=...)ax.legend(*sc.legend_elements(**legend_viz_params))

or let the artist take the required parameters

sc = ax.scatter(x,y,c=.., s=..., **legend_viz_params)ax.legend()

or let the legend take them,

sc = ax.scatter(x,y,c=.., s=...,)ax.legend(**legend_viz_params)

Or does it?
In any case, if that proposal to give artists a generalobtain_copy_of_me_for_legend (or__legend_repr__) method and let the legend use it is to be pursued further it would kind of revolutionize theLegend and replace the existingHandlers. Would it make sense to discuss this in a separate issue? I only fear that such issues will just end up never being implemented and step-like improvements like the inital proposal here are being blocked on them.

I wasn't exactly sure the scope of what legend_handler could do.

I agree that the functionality here is useful, and I don't really see how it can easily be made extensible.

What I'm hung up on is that I am not sure I like the API of having it a method onPathCollection. Thats not where I would look for it, and I think makes it obscure. Would callingax.scatter_legend(sc) be too restrictive? One could even imagine passing it other handles and labels if you wanted more than just the scatter info on the legend:ax.scatter_legend(sc, h_add, l_add, **otherkwargs).

OTOH, if we do this for every different artist type, I imagine this gets to be a mess so I can see the point of having it attached to the artist.

Again, just trying to think the API through a bit. I think this is a good idea overall.

ax.scatter_legend(sc, mode=.., useall=.., num=.., .. ,**kw) is totally possible. It would make the separation between legend kwargs and kwargs needed to steer the specific scatter legend output easier. I would agree to it getting messy if further down the road many similarax.<plottype>_legend() methods were to be added.
It also goes in a completely different direction than the proposal of anArtist method by@tacaswell.
Following his suggestion,

I think for now coming up with a consistent name and API for Artists to return a list of labels and handles as an optional public method is a good start

we currently have:

• Artist.legend_elements() (This PR)
  • Pro: name is consistent with existingContourSet.legend_elements
  • Con: name has no verb, hides the non-unique nature of the return
• Something likeArtist.{create/make}_legend_{items/handles/elements}()
  • Pro: could better reflect the process of producing something from the artist.
  • Con: inconsistent with existing contour method; adjusting contour method would break existing API
• Artist.get_legend_handles_labels()
  • Pro: consistent with existingAxes.get_legend_handles_labels()
  • Con: inconsistent with existing contour method; adjusting contour method would break existing API, rather long name.

Concerning the API, i.e. the needed arguments, I think there might be little to no overlap between what different artists need as input. Evennum (the number of created legend elements) is not really needed for many artists, for which anything butnum=1 does not make sense at all.

Sorry, I missed the point aboutlegend_elements already being used forcontour. And I see that is a method onContourSet. Given that precedent, I'm fine w/ the general level of the API here.

Update: You may now pass a locator tonum.

Concerning the discussion about having alegend_elements method forArtist, I think one may do something like

class Artist(object):    # ...    def legend_elements():        return [self], [self.get_label()]

However, as long as this isn't used anywhere internally, it's not really useful. In any case this can be part of a different PR, in case one wants to letlegend use this for obtaining its handles/labels.

@ImportanceOfBeingErnest we talks about this on the weekly phone call, and the issue is with attaching this method toPathCollection which is a more general class than just for scatter.

The request would be that we add aScatterArtist subclass of PathCollections that has extras like this as new methods just on the subclass, and of course change the call toscatter to use this new subclass. Thats a somewhat bigger job than just this PR, but should be manageable and would make other things easier; the object returned from scatter really should have been more than just aPathCollection earlier than this.

If you don't feel like doing this, I imagine someone else will be up for picking it up! Thanks for a great idea...

But this is not only useful for scatters. If you have a different PathCollection than the one being created by a scatter, you may still use it to create legend entries for it. As of now, it seems no other matplotlib function other than scatter creates PathCollections, but if there were others, why restrict this functionality to onlyScatterArtist?

What's the status here?
Should there really be a newScatterArtist created which then inherits thePathCollection'slegend_elements? Or is there any profound reason to only make this method available for aScatterArtist even though it's perfectly usable for aPathCollection as well?

Myunderstanding which may be incomplete, is thatPathCollection is meant to be a more general path container and hence we want to not have any methods on it that are specific toscatter. Yes,PathCollection is only currently used forscatter, but thats not the intended end-use case.

If you still disagree with this organization, then we can bring other folks into the conversation.

As said, this is not specific toscatter - although clearly motivated by it.
If you can show me aPathCollection for which the newly added method would fail, that would all make sense. One could also argue that for the most general case one should add a third option asmode argument, which would create legend entries for several paths. But in that case, the method would still reside inPathCollection, not in an inherited class, right?
At the beginning, a question asked how farup the hierarchy this should sit. Now suddely the question is how fardown it should be moved.
Just to be clear here: I'm not opposed to add aScatterArtist at all. But even if doing so, I would still let thelegend_elements method stay inPathCollection.

I don't have any skin in this game - you'll have to work it out w/@tacaswell

ping@tacaswell

Marking as "Needs Review" to hopefully burble to the top of people's lists...

Looks like doc build and travis aren't happy at the moment?

I'm happy to fix all the little hickups that come and go with updated dependencies (pep8 to flake, sphinx 1.7 to 1.8 etc.) once the general fate of this PR is decided upon.
I will not waste my time continuously keeping up with updated depenencies in order to have the checkmarks green every day for the next 5 months, just to then have someone decide that this is not the route to go at all.

... is this really release critical? No, but I can imagine@ImportanceOfBeingErnest is a bit frustrated its sat around for months...

I am 👍 on this,@efiring is responsible for getting this merged.

Thanks for the review@efiring. I think I have now incoorporated all the requested changes.

I think it's good to have the API fixed for the case of the scatter (which is the most common usecase I assume). We can now still implement this for other collections. If you want I can already now put aNotImplemented into the generalCollection.

Looks good, so I merged it. I don't think that putting the method into the Collection base would be good, because it probably makes sense to implement it only for some types of Collection.NotImplemented would imply that itshould be implemented for everything, which I don't think is the case.

Thanks@efiring and thanks@ImportanceOfBeingErnest for your patience with this!

🎉 I am glad this landed!