Looks like (at least) figlegend is going to cause issues :/.

Apparently, using a markevery value different from None value triggered an new code path (for that situation) where an axes transform (ax.transAxes) is assumed to be available.

23503e5 is a crude fix that simply uses self.get_transform() if the aforementioned hypothesis breaks. I do not totally understand (not to say at all) why it seems to work, but at least locally it fixes the error that was occurring with the figlegend demo...

Well, I cannot reproduce the Travis failures (of test_markevery_line and test_legend_auto3) on my local computer :/.

Any reason to make this new handler private? All other handlers are public such that they can be directly used by the user or subclassed, if needed.

I wanted to avoid having two public handlers for Line2D objects (“There should be one-- and preferably only one --obvious way to do it.”) as this tends to end up in fuzzy situations for end-users, like in the case for arrows. But I was also unsure about how heavily used is the current HandlerLine2D outside of our codebase, which would give clues about the relevance of deprecating it in favor of a more monolithic approach (like the one in _HandlerMonolithicLine2D).

AFAICT, a private new handler is a cheap way to avoid those issues while still fixing #11357 . We could always decide later to make the new handler public and/or deprecate the previous one, could we not? (But maybe I am wrong; API decisions are not my field of expertise ^^)

I would rather see this similar to e.g. all the Locators and Formatters. They are all public and some of their functionality overlaps. The user just chooses the one they like and matplotlib decides for a default one.

I have to say I am also frequently surprised about what kind of API changes that truely break things are simply put in without notice while other seemingly straight forward improvements are heavily disputed.

So the following is just my personal view of things:

• You are changing the API with this proposed PR. So you need to make sure it produces the same results as before (modulo some half pixel offset one wouldn't care about).
• The new handler should therefore be public and usable by people.
• The old handler should stay for people who need it. (Possibly rename it to something else.)
• I supect that this change will break some existing code, namely those where people worked around the fact that there is only one of the two created artists returned. Since this code change is for the better, people are hopefully happy to adjust their code to the new handler.
• This change needs to be documented in the API changes.

If the goal is ultimately to kill the old handler in favor of the new one, I would suggest implementing this as a (new) rcparam switch in create_artists. This would make the ultimate API switch much easier to manage, and (I think) in the meantime people would be able to use the new API by globally setting the rc, or even locally with

with rc_context({"legend.linehandler": "monolithic"/"nonmonolithic"}): ...

(names up to bikeshedding) just around the call to legend().

(-1 on "renaming to something else", that's how we end up with OldAutoLocator which been around for more than 10 years)

Oula that would grow big now... if you introduce a rcParam for legend handlers the ultimate expectation would be to define any legend handler through the rcParams. I think this would be useful, but might exceed what is attempted here.

My argument from above was more to do this API change right now, because as I see it, the only thing this would break is code written to overcome exactly the limitation that the new handler fixes.

(About name bikeshedding, before I forget: "nonmonolithic" <- "composite" or "compound")

Actually the point was not about making handlers rc-customizable, but to make API changes more controllable (perhaps instead the key can be something like api.legend_line_handler, and likewise for any API change which is accompanied by a change of default behavior).

1. I think the original API didn't make much sense and that this should be the HandlerLine2D. Yeah, it'll break some code, but I can't see the situation where someone was depending on the old behaviour who can't achieve what they want more easily w/ the new behaviour. I don't think the handlers have such a huge user cross section that we'd get a lot of blowback from this API change; but I'm not as conserative as I probably should be on such things.
2. If we do decide on a new name rather than just using the old, "monolithic" is a bit overbearing for something that just combines two handles into one, and will appear quite mysterious to new users. HandlerSingleLine2D? HandlerCombinedLine2D?

If you want to revert to the old behaviour you can easily add

from matplotlib import legend, legend_handler, pyplot
legend.Legend.update_default_handler_map({pyplot.Line2D : legend_handler.HandlerLine2DComposite})

(Edited.)

at the beginning of your scipt. I don't see any necessity to add a single rcParameter which can only be used to change the legend handle for lines but not for other artists.

Just to make sure we're all aware of the implications here. Previously if you wanted to e.g. change the marker color of a line in a legend you'd need to do

leg = ax.legend()
leg._legend_handle_box.get_children()[0].get_children()[0].get_children()[0].get_children()[1].set_markerfacecolor("red")

with this PR you can just do

leg.legendHandles[0].set_markerfacecolor("red")

Everyone affected by this will love the new possibility and all I'm asking for here is to make the new handler public, just as all other handlers, and to add an API change note. I would also find it usefull not to remove the old handler from the code base, simply because we cannot know for sure if some people would not like it the old way still, maybe because they have subclassed the handler for some other custom thing.

Well, manipulating private attributes is not great.
But perhaps we can indeed thing about publically exposing _default_handler_map.

@anntzer I think I have something all-public (mlegend.Legend.update_default_handler_map). (rebase incoming)

Overhaul based on the comments above (thanks all). I'll update the initial post.

I would totally favour making _default_handler_map public, but for completely different reasons: Namely to easily "register" a custom legend handler globally, instead of passing a dictionary to each legend individually.
(Edited: This does not apply. You can use update_default_handler_map publically.)

I may be a bit out of my depth here, but mlegend.Legend.update_default_handler_map is a class method, so does it not already do that globally?

(Rebase because I think I forgot a line break in the API note and Sphinx was complaining.)

The recurrent failures of test_markevery_line and test_legend_auto3 on Travis are still around, but I am not able to reproduce them locally :/.

Edit: Moreover I still have a couple things to figure out about Sphinx subtleties in the API change note (like how to show the example plot & its source code, and manage to get :ghissue:11358 working...)

Huh, they are just svg tests that are off by 0.001 and 0.004. My guess is the lines or markers moved a tiny bit in svg land because you are drawing them a bit differently. I'd be inclined to remove the svg tests for those, or increase the tolerance. You may not do the svg tests locally?

Looking at the SVG files that are locally generated (but do not trigger test failures here :/), the only differences (apart from IDs all along the files) are the additions of z instructions in a few places (in comparison to the expected SVG file). This SVG instruction apparently stands for “closepath”, but I do not know how it modifies the actual final rendering in this case.

In order not to entirely drop the SVG tests for the two aforementioned “failing” tests, I increased the tolerance following the scheme current RMS error + 0.001: let's now see if Travis is happy.

Edit: Before I forget (as this does not seem to trigger failure), a couple Sphinx subtleties still have to be fixed in the API change note (like how to show the example plot & its source code, and manage to get :ghissue:11358 working...) I found the issue with :ghissue and I gave up on the idea of displaying both an example plot and its source code in the API note (in favor of simply showing the self-sufficient example snippet). Rebased to squash the commits that were related to the API note.

@anntzer Done :) (although the message(s) may be a bit verbose). Being there, rebased too.

@anntzer Is the current version fine for you now? (shhh, this is an undercover ping ;))

ping @afvincent (or we should punt to 3.1)

Rebased and merged in #20699. Thanks @afvincent and @anntzer !