The same piece of code exists in imshow() (with the same doc as for scatter). Does it also need to be fixed there?

There's a few others which may need the same treatment (haven't fully checked) -- hexbin, pcolor, pcolormesh, pcolorfast. Maybe worth extracting into a separate function?

This is now a private methodScalarMappable._scale_norm. While, theoretically, it could be useful for third-parties as well, I'm not 100% sure on the naming and scope of the method. Therefore, I'd leave it private for now.

The whole mappable initialization is a bit of a beast because the data are not part of the constructor and scaling depends on the data. Might be possible to untangle that further but that's for another PR.

Should the change in behavior (even if arguably a bugfix) be documented?

Should the change in behavior (even if arguably a bugfix) be documented?

Not sure. Do we document bugfixes? By their nature they change the behavior. But they are not an API change with respect to the intention. Usually people won't have to change their code because of that. If we want to document still, we should have a seprate bugfixes category.

Actually, taking a step back... isn't this going to break people who are e.g. callingimshow(..., norm=LogNorm(), vmin=someval, vmax=someotherval)? Admittedly they could just pass vmin/vmax to the LogNorm constructor (so perhaps breaking them is fine -- with the proper warning yada yada), but I don't think such cases are rare?

isn't this going to break people who are e.g. callingimshow(..., norm=LogNorm(), vmin=someval, vmax=someotherval)?

Yes. So to be defensive we want:

• imshow(..., vmin=a, vmax=b) - ok
• imshow(..., norm=LogNorm()) - ok
• imshow(..., norm=LogNorm(a, b)) - ok
• imshow(..., norm=LogNorm(), vmin=a, vmax=b) - deprecated, but scale; useLogNorm(a, b) instead
• imshow(..., norm=LogNorm(a, b), vmin=c, vmax=d) - deprecated,should we scale because that is the current behavior or not scale because that's what the documentation says? still scale because that's the current actual behavior.

Actually, now that I think of it again, it seems a bit gratuitious to forbidnorm=LogNorm(), vmin=..., vmax=.... I think the only thing that needs to be warned against is if the normalready has vmin or vmax setand vmin is passed separately as well. (As to the behavior in that case, I think it's better to just keep what the current behavior is (assuming it is indeed consistent across plotting methods...) and adjust the doc accordingly.)

I feel a bit different aboutnorm=LogNorm(), vmin=..., vmax=....

My interpretation of a purevmin=..., vmax=... is that it's a convenience shourtcut for "create Normalize with these limits" without having to explicitly importNormalize. On that background,norm=LogNorm(), vmin=..., vmax=..., feels a bit odd. It does not create a norm but modifies the existing norm (but then again only if that norm has no explicit limits). Stating this correctly in the docstring is cumbersome, and IMHO a hint that it's not a good API. Moreover, it is longer and less clear thannorm=LogNorm(vmin=..., vmax=...).

Side remark:vmin,vmax would actually not have been necessary. One could have had a clearer interface that acceptsnorm=(vmin, vmax) as a shortcut fornorm=Normalize(vmin, vmax). This would not have raised this issue that multiple parameters affect the same logic and we have to make sure all combinations of these parameters are handled reasonably. However, I acknowedge thatvmin, vmax are common and widely used parameter and thus should stay for the case thatnorm is not given.

I agree norm=(vmin, vmax) would have been nice but that ship has sailed. I guess I'm -0 on deprecatingnorm=LogNorm(), vmin=..., vmax=... but am not even close to wanting to block it either.

I've decided to move forward with deprecating simultaneous use ofnorm andvmin/vmax. Let's see what others say.

Actually looking at this again (^2), if we want to go in this direction, perhaps (in a separate PR) we can indeed add support for norm=(vmin, vmax) and "soft-deprecate" vmin=..., vmax=...? (hide it behind kwargs, don't mention them in the main docs but only as an aside to the docs of norm?)

perhaps (in a separate PR) we can indeed add support for norm=(vmin, vmax) and "soft-deprecate" vmin=..., vmax=...? (hide it behind kwargs, don't mention them in the main docs but only as an aside to the docs of norm?)

I would support that direction, but that's definitively something for another PR. Also the namenorm is a bit too technical. If I was to design this anew I would propbably go for something likecscale in resemblance tocmap. It would need more discussion if we see benefit in larger a soft-deprecation to clean up the API.

For this PR, let's just make the existing API behave consistently and match the documentation.

vmin=a, vmax=b is extremely ingrained... I know what a norm is now, but found the concept pretty confusing when I started using matplotlib, not helped by the fact it was almost completely undocumented until I wrotehttps://matplotlib.org/3.1.1/tutorials/colors/colormapnorms.html. So I think it'd be tough to getvmin andvmax deprecated.

Given that, I'm somewhat skeptical about disallowingnorm=LogNorm(), vmin=-3 etc. though I guess thats somewhat more justified.

This is a case were we are going to have to be sensitive to any protest from users.

Agree with@jklymak that deprecatingvmin andvmax will be a hard lift, and one I am not sure we should try. I have frequently had the use case were I want to set just the min or just the max which thenorm=(vmin, vmax) formulation makes awkward.

Let's see if this deprecation causes user feedback.

I'm not sure either if we finally want to deprecatevmin,vmax. That could be discussed some time in the future. I have no actions planned in that direction so far.

FWIWnorm=(None, 42) should work? (I'm not claiming it's a great API though.)

@anntzer sure, but that is a bit annoying.

This is one of those application vs library balance discussions (and I'm leaning towards the application side).

Actually, one thing I had missed is that there's also alreadyimshow(..., clim=(vmin, vmax)). In particular,imshow(..., clim=(...), norm=LogNorm()) currently doesn't warn. Should it, in fact, emit a warning?