Fix calls to np.ma.masked_where #20511
Conversation
For the same reason as Quiver in matplotlib#4250.
076c4e6 to
32d100e
Compare
| @@ -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.array(A, mask=(A <= 0))) | |||
There was a problem hiding this comment.
Does this save the copy step? It seems like this is creating a new array anyways, so I'm wondering if it is gaining anything relative to just removing the copy=False?
There was a problem hiding this comment.
The default for np.ma.array is copy=False, keep_mask=True. The former will not copy the data, and the latter will use the existing mask (if any) with the new mask, which will make a copy. Removing copy=False from masked_less_equal makes a copy of both.
There was a problem hiding this comment.
Thanks for clarifying! I was curious how to test if this actually holds and learned about np.shares_memory today. I did verify this assertion with that.
There was a problem hiding this comment.
You can also check, if you have b = np.ma.array(a), that b.base is a (if a is originally a plain array), or b.base is a.base (if a is originally a masked array).
32d100e to
d733f9f
Compare
NumPy 1.21.0 fixed a bug with `np.ma.masked_where` where `copy=False` now modifies the input if it is masked, which we do not want to do. `np.ma.array` defaults to not copying the data, but copies the mask (adding the new mask), which is what we wanted with `copy=False`.
d733f9f to
48eef46
Compare
…511-on-v3.4.x Backport PR #20511 on branch v3.4.x (Fix calls to np.ma.masked_where)
|
@QuLogic a small note that I think this change can cause a bug when the array We ran into this with GeoPandas (geopandas/geopandas#2066, didn't test it, but I assume this PR is the cause), where we were (incorrectly) setting the array of a color map with a list Just noting it here, I'll let it to your judgement whether this is worth addressing or not (it's certainly not needed just for GeoPandas). |
|
Just a note that I am pretty sure this is fixed in master due to a new call to a call to matplotlib/lib/matplotlib/cm.py Line 459 in ca275dc v3.4.3 >>> sm.set_array([])
>>> sm.get_array()
[]master >>> sm.set_array([])
>>> sm.get_array()
masked_array(data=[],
mask=False,
fill_value=1e+20,
dtype=float64)I don't know if another 3.4.x release is planned sometime, but it should be a pretty simple fix to add that call in to cast to array immediately in the set_array call. |
|
That came in from #18870, which I don't think we can backport fully. But we can make a small correction to this, as it's not required that |
PR Summary
Due to numpy/numpy#18967, I've audited all our calls to
np.ma.masked_*(copy=False). When the input comes directly from the user, or is already stored somewhere already, then we don't want these calls to modify the existing values.This affects
np.ma.masked_whereand all simple wrappers around it likemasked_less_equal. It does not affectmasked_invalid, which continues to not modify the input. I have asked upstream numpy/numpy#19332 whether that is intended. If they intend to changemasked_invalidalso, then we will need to make a few more changes like this, but we are mostly okay.PR Checklist
pytestpasses).flake8on changed files to check).flake8-docstringsand runflake8 --docstring-convention=all).doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).