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 method ScalarMappable._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. calling imshow(..., 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. calling imshow(..., 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; use LogNorm(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 forbid norm=LogNorm(), vmin=..., vmax=.... I think the only thing that needs to be warned against is if the norm already has vmin or vmax set and 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 about norm=LogNorm(), vmin=..., vmax=....

My interpretation of a pure vmin=..., vmax=... is that it's a convenience shourtcut for "create Normalize with these limits" without having to explicitly import Normalize. 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 than norm=LogNorm(vmin=..., vmax=...).

Side remark: vmin, vmax would actually not have been necessary. One could have had a clearer interface that accepts norm=(vmin, vmax) as a shortcut for norm=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 that vmin, vmax are common and widely used parameter and thus should stay for the case that norm is not given.

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

I've decided to move forward with deprecating simultaneous use of norm and vmin/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 name norm is a bit too technical. If I was to design this anew I would propbably go for something like cscale in resemblance to cmap. 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 wrote https://matplotlib.org/3.1.1/tutorials/colors/colormapnorms.html. So I think it'd be tough to get vmin and vmax deprecated.

Given that, I'm somewhat skeptical about disallowing norm=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 deprecating vmin and vmax 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 the norm=(vmin, vmax) formulation makes awkward.

Let's see if this deprecation causes user feedback.

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

FWIW norm=(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 already imshow(..., clim=(vmin, vmax)). In particular, imshow(..., clim=(...), norm=LogNorm()) currently doesn't warn. Should it, in fact, emit a warning?