BUG: add missing MPL version check to a temporary hack introduced in #3161

[neutrinoceros]

PR Summary

This is a proposal to close #3170 by adding a version check for MPL so that the "hack" introduced in #3161 is only applied when strictly necessary (with matplotlib from 3.3 to 3.4)

Taking the example from the original issue #2890 for reference

import yt

ds = yt.load_sample("FIRE_M12i_ref11/snapshot_600.hdf5")
yt.ProjectionPlot(ds, "x", ("gas", "density")).save()

Here are the results:

Main branch

w/ Matplotlib 3.2 to 3.4

w/ Matplotlib 3.5

(the one difference is that minor colorbar ticks are drawn, due to unrelated changes upstream)

This branch

w/ Matplotlib 3.2

w/ Matplotlib 3.4

w/ Matplotlib 3.5

So my main motivation here is to make it clear when (if) we can drop the hack introduced in #3161 in the future, but
I also think the behaviour observed with this patch + Matplotlib 3.5 is more desirable from the user's standpoint.
I acknowledged that a counter argument would be that the current behaviour (main branch) is preferred because it's more consistent across matplotlib versions.
So I'd like to ask @chummels @jzuhone and @chrishavlin to weigh in and decide if this patch is desirable or if we should just close #3170 without a code change.
Of course everyone else is also welcome to provide feedback

[chrishavlin]

Great! I'm not opposed to the version-dependent behavior here.

I am curious if you've experimented with the new imshow keyword parameter (interpolation_stage)? I'm actually a little surprised the This Branch with mpl 3.5 case didn't error as I thought it would be using the standard imshow internal scaling (and so would expect the same roundoff error from before). But maybe there were upstream changes there too?

I'm happy to approve this but will wait for others to have a chance to weigh in as well.

[neutrinoceros]

No I haven't tried the new parameter yet. I think I'm secretly hoping we do not need it anywhere so we have less version-sensitive code to maintain.

And yes, let's wait for at least one more approval from either John or Cameron before we consider this for merge

[chrishavlin]

think I'm secretly hoping we do not need it anywhere so we have less version-sensitive code to maintain.

I totally agree there.

[chummels]

I'm very happy to see that MPL updated to fix that problem with the zeros erroring out! Thanks for staying on top of this, @neutrinoceros . I certainly don't love my fix in #3295 that forces the symlog version of this plot to have all the orders of magnitude listed on the colorbar, as it's pretty cumbersome. The default that we had before and can now again work with MPL 3.5 is much cleaner. I'm a little bit hesitant to have such version-dependent behavior, but I think it probably makes sense in this case. I'd like to get feedback from @jzuhone or @matthewturk or others on this before we merge it though.

[neutrinoceros]

Thanks for your feedback !
So that everyone is on the same page I'd like to give my idea of the bigger picture here, as I've been maintaining many other such hacks in yt that are MPL-version dependent: MPL 3.3 and 3.4 series have been particularly disruptive for yt plots, so I tend to see them as "cursed" releases and I prefer having yt plots look slightly different in these versions rather than letting them dictate how yt plots should look against any MPL version. Again this is just my opinion.

[neutrinoceros]

Moving this somewhat aggressively to the 4.0.3 milestone to make sure it's at least discussed. I'd be okay with not included it in the release if anyone wants to oppose.