If people are happy with this (which seems to be the case) I still need to chase through the docs to make sure they are still correct.

I believe this suffers from the same underlying problem that caused the old implementation to be so roundabout: some keys are not hashable because they are mutable. To be concrete, if you write e.g.

lims = [.1, .1, .9, .9]ax1 = fig.add_axes(lims)lims[0] = .2ax2 = fig.add_axes(lims)ax3 = fig.add_axes([.1, .1, .9, .9])ax4 = fig.add_axes([.2, .1, .9, .9])

what are the axes that you think should be the same? (A similar, and probably less artificial, argument applies e.g. foradd_subplot(111, projection=some_mutable_thing).

(Holding onto kwargs can be useful, but I think this more or less requires (in this specific case) enforcing that projections be immutable.)

@anntzer, I'm not following - first this PR doesn't do anything for add_axes etc. it only affectsplt.subplot, and the only danger of

plt.subplot(projection=myMutableProjection)myMutableProjection.mutate()plt.subplot(projection=myMutableProjection)

is that they will lose the original plot because the projection has changed, in which case they should just keep the reference:

ax = plt.subplot(projection=myMutableProjection)myMutableProjection.mutate()plt.sca(ax)

That seems pretty esoteric to me. But it is certainly better than breaking the people who expect the axes to clear when doing

plt.subplot(projection=proj1)...plt.subplot(projection=proj2)

I wrote the example with add_axes for simplicity, but again something similar can happen with projections if they are not immutable. I don't actually know much about cartopy or WCSAxes, but I don't think it would be incredible to have some projections library that supports e.g.proj.set_central_longitude(whatever) (for example), in which case you run into the same issue (proj = Proj(); subplot(proj); <sometime later>; proj.set_central_longitude(whatever); subplot(proj) which doesn't seem so unlikely to happen).

And if the project is actually immutable, then it should normally be hashable too, so that would be one thing to check.

proj = Proj(); subplot(proj); <sometime later>; proj.set_central_longitude(whatever); subplot(proj)

right, and with this change, the second subplot will overwrite the first. But a) thats hopefully rare, and b) when that happens, presumably its pretty obvious, and we can just tell people to save the reference to the subplot, rather than fragile grabbing from thesubplot stack

the second subplot will overwrite the first

I believe that what will happen is that the second call will return the preexisting axes as is?

presumably its pretty obvious, and we can just tell people to save the reference to the subplot

Just to be clear, it is far from obvious to me what people using the implicit machinery may be expecting, because it's been too long since I've last used it for this kind of manipulations, but heh.

The key behavior change in#19153 (and what caused the re-write from#18978 is that we moved the re-use or new (and remove) logic into pyplot from theFigure methods. I think everyone is still on board withadd_subplot doing what it says on the tin and always adding a new Axes to the Figure. I think it is fair to ask people who are using the explicit (nee OO) version of the API to explicitly keep track of their Axes(es) and the only behavior still up for discussion is whatpyplot does.

The argument for keeping the "create or query" logic in pyplot is that if the users are going fully down the implicit route (a number of years ago I had an lengthy discussion with a user who was convinced beyond doubt that the implicit API was easier and clearer to use, 30-45 minutes of effort and several drinks had no effect) then this is the only way for them to come back to an Axes via the internalsca call (which I discovered while writing this up was broken so fixed that too).

If we have a well behaved non-mutable input (or the users just never mutate it ;) ) we have:

obj = SomeClass()ax1 = plt.subplot(111, keyword=obj)obj2 = SomeClass()ax2 = plt.subplots(111, keyword=obj2)

• on 3.3 this returns a new Axes and removes the existing one
• on master this return the first Axes because we ignore the kwargs when doing the matching (which@astrofrog makes a good case is a major API change, I think we have to back out of one way or the other)
• with this patch it return a new Axes and remove the existing one.

However@anntzer is correct that this is opening is up to some new odd behavior when the values are mutable. Python is too flexible to reliably tell if something is mutable (and even checking if it is hashable is not technically enough as you can over-ride__hash__ is questionable ways).

obj = MutableClass()ax1 = plt.subplot(111, keyword=obj)obj.mutate()ax2 = plt.subplots(111, keyword=obj)

• On 3.3, this gives you back the same Axes or fail to create the first axes depending on ifobj is hashable.
• On master we fully ignore the kwarg the second time through and return the existing axes
• With this patch we still get back the same Axes because we decide that the kwargs are equal

Reusing the axes may or may not be the right thing because it will end up depending on how the projections work (which is between the objects being based in and the project code we have discovered). If they look at what ever state is inobj and draw time, then getting back the same Axes is correct, if the state is used at Axes creation time then this behavior is wrong.

We could make a shallow copy of all of the values, but that would only push the problem "down a layer" or make things wrong in the opposite case as above. We could make a deep copy which may be expensive and inverts the cases where the behavior is wrong.

There is already going to be an issue the values are numpy arrays ("truth value of an array is ambiguous!"), but they also didn't work in <=3.3 so I am not worried about that

I think on net, the behavior in this patch is the best option (modulo adding some docs). In addition we could:

• always warn in pyplot with kwargs, but that would be annoying to polar or 3D users (who are giving us string values).
• explicitly allow some known OK keys / values and warn on anything else
• try to hash the values as a hokey test

I'm inclined to leave it as-is and not get too deep on the semantics of this behavior with the "fix" for user problems is "please use the explicit API where you have a variable name to keep track of what Axes you want to use rather than a proxy variable name through some shared state".

OK, I misunderstood@anntzer. The problem is that the user might mutate the object and think that they should get a new axes when they call subplot the second time. I guess I am OK with that being squishy and I still stand by my approval of this approach...

I am now abit worried aboutpyplot.axes as well.

Per the above I don't really know what's best for plt.subplot() users, so as long as the implications with mutable args have been thought out I'm fine with that (I agree with@tacaswell fig.add_subplot should just always add a new axes).

I have confirmed that, if merged, this will fix the issues in Astropy. Seeastropy/astropy#11315.

This looks good to me, thanks@tacaswell!

See also#10700.

OK,this time I think I got enough of the corner cases. It is a fair bit of code, but I think we have gotten all of the logic layed out in one place now :) Most of the line count is additional tests.

Simplified the implementation and line-wrapped the comments to 79 characters.