Yeah I don't think this fix is correct. I debugged for a while and self.vmin changes from 100 to 0 in the offending test. I have zero understanding of where that may have happened, but I think this is hiding the problem.

s_vmin and s_vmax are what temporarily change norm.vmin and norm.vmax. That code came in at the same time as the test, in #18458. The intention is to re-scale things to not go out of bound somewhere or other, but this block ensures that we don't rescale too much and go out-of-bounds for log.

The difference seems to be with A in _make_image, at least by line 396 or so, but I think it's not modified before that. In NumPy 1.17.5, the array contains 1e20, and -1, but in 1.21, the array is masked. After scaling, the -1 is still -1, so it easily matches the condition before the change above, and is replaced by epsilon. When it's masked, the min and max of the array are 1e20, and eventually that works out to 0 at the end of the re-scaling stuff. I haven't worked out why the mask is added/dropped in one version or the other.

I ran this through a debugger and A has the negative values masked in the new version of Numpy, whereas before the mask was left off (a_min is -1 previously, 1e20 now). I think the offending line is:

I'll try and look more into this later today.

I think this PR fixed the old behavior where the MaskedArray wasn't being modified even with a copy=False: numpy/numpy#18967

We can fix this test by creating a copy, which is the default in numpy. Alternatively, do we want the autoscaling function to modify the passed in array and mask elements or do we want autoscale to only set the vmin/vmax and never modify the input array? I can see both sides...

✔ 15:56:17 $ git diff
diff --git a/lib/matplotlib/colors.py b/lib/matplotlib/colors.py
index e0c42c5b69..5e9c6f6dbd 100644
--- a/lib/matplotlib/colors.py
+++ b/lib/matplotlib/colors.py
@@ -1545,11 +1545,11 @@ class LogNorm(Normalize):
 
     def autoscale(self, A):
         # docstring inherited.
-        super().autoscale(np.ma.masked_less_equal(A, 0, copy=False))
+        super().autoscale(np.ma.masked_less_equal(np.ma.getdata(A), 0))
 
     def autoscale_None(self, A):
         # docstring inherited.
-        super().autoscale_None(np.ma.masked_less_equal(A, 0, copy=False))
+        super().autoscale_None(np.ma.masked_less_equal(np.ma.getdata(A), 0))
 
 
 @_make_norm_from_scale(

This patch fixes the tests

That code appears to have not 'made a copy' for 11 years: 3101e00 I think the intention was don't copy if-not-needed (and not as the NumPy docs intended), but it's hard to say now. It's also possible that our input changed from maybe-masked to always-masked with the changes to how input data is (not/now) copied, and that had this side effect we didn't expect.

That patch will remove already masked values that were present though and it removes the copy, so I'd suggest just removing the copy=False instead of adding the getdata call.

🤦🏻 meant to leave the copy...

diff --git a/lib/matplotlib/colors.py b/lib/matplotlib/colors.py
index e0c42c5b69..3766e57d03 100644
--- a/lib/matplotlib/colors.py
+++ b/lib/matplotlib/colors.py
@@ -1545,11 +1545,19 @@ class LogNorm(Normalize):
 
     def autoscale(self, A):
         # docstring inherited.
-        super().autoscale(np.ma.masked_less_equal(A, 0, copy=False))
+        super().autoscale(
+            np.ma.masked_less_equal(
+                np.ma.getdata(A), 0, copy=False
+            )
+        )
 
     def autoscale_None(self, A):
         # docstring inherited.
-        super().autoscale_None(np.ma.masked_less_equal(A, 0, copy=False))
+        super().autoscale_None(
+            np.ma.masked_less_equal(
+                np.ma.getdata(A), 0, copy=False
+            )
+        )
 
 
 @_make_norm_from_scale(

also passes. I think we want to try to avoid making needless (internal) copies of image data which can be big.

And I think we still need a version of this PR to fix a different bug where the vmin of the norm is very far from the minimum of the data.

On the phone @QuLogic convinced me that @greglucas is right and we want to a) not copy the underlying data b) preserve any existing mask the use gave us c) enrich that mask with where the data is less than 0.

This seems like a good bit of dancing around just so we can call super()?

Sorry, I should elaborate. I don't think you want to permanently add to the users mask, because you can't go back to a different norm at that point.

We are still not messing with the users copy (we make a copy earlier in the process), but with numpy 1.21.0 we are mutating our internal copy of the mask.

But we don't re-supply ourselves with the user's copy, so we would be blanking out all negative data if we change the norm on the scalarmappable with set_norm?

OK, I think we should leave the autoscaling copy=False alone and use it as it currently is to preserve the passed-in masks and not copy anything as mentioned in comments above.

The issue was that before our s_vmin was being set to -1 because the array was populated with -1's. If we had populated the array with 0's we would have been in the same boat as now.

I added another test to duplicate this behavior, except using a full range of 1 -> 1e20 by setting the array to 1's initially to verify the actual large range case gets covered too.

I'm not following what the other bug with copying is actually? I think the copy=False is doing what we think it is supposed to do now, it is just this test got unlucky because of < catching the negative values before that aren't there anymore.

I'll open a PR for that soon.