Mplot3d masked surface #18114

tomneep · 2020-07-29T11:57:22Z

PR Summary

This allows np.ma.MaskedArray to be used as the data in plot_surface().
I expect that the solution is the same as the one proposed in #487.
The colormap min/max is normalised to the data that is shown in the plot, as is the zlim.

Demo

Taking an example inspired by that of #487 produces the following

import numpy as np
import matplotlib.pyplot as plt 

x = np.arange(1,10,1)
y = np.arange(1,10,1)
x,y = np.meshgrid(x,y)
z = x**3 + y**3 - 500
# To test what happens when one of the masked values
# is far off the other points (z-axis range, colormap, etc)
z[0, 0] = -1e9
z[-1, -1] = 1e9
z = np.ma.masked_array(z, z<0)
z.mask[-1, -1] = 1

fig = plt.figure(figsize=(8, 8))
ax1 = fig.add_subplot(221, projection='3d')
ax2 = fig.add_subplot(222)

surf = ax1.plot_surface(x,y,z,
                 rstride=1, cstride=1, linewidth=0,
                 cmap='inferno')
plt.colorbar(surf, ax=ax1, shrink=0.7, orientation='horizontal')

ax2.imshow(z, cmap='inferno')

fig.show()

PR Checklist

Has Pytest style unit tests
Code is Flake 8 compliant
New features are documented, with examples if plot related
Documentation is sphinx and numpydoc compliant
Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
Documented in doc/api/next_api_changes/* if API changed in a backward-incompatible way

lib/mpl_toolkits/mplot3d/axes3d.py

lib/mpl_toolkits/tests/test_mplot3d.py

lib/mpl_toolkits/mplot3d/axes3d.py

dstansby

Thanks for opening the PR! I've added a couple of comments/questions, but this looks 👍 overall

dstansby · 2020-09-20T16:14:16Z

lib/mpl_toolkits/mplot3d/axes3d.py

@@ -1559,8 +1559,8 @@ def plot_surface(self, X, Y, Z, *args, norm=None, vmin=None,
                "Z contains NaN values. This may result in rendering "
                "artifacts.")

-        # TODO: Support masked arrays
-        X, Y, Z = np.broadcast_arrays(X, Y, Z)
+        mask = np.ma.getmaskarray(Z)


What happens if X or Y have a mask? My guess is we should take the union of all the masks to compute an 'overall' mask here.

Good question, I see that pcolormesh doesn't accept masked X and Y values (but it looks like pcolor might). I've not tested contour but it looks like masks are ignored on X and Y values?

lib/mpl_toolkits/mplot3d/axes3d.py

sdbbs · 2021-04-09T15:24:46Z

I would just like to mention: patching this PR with the following hack (easiest I can do is diff in shell code):

$ wget https://raw.githubusercontent.com/tomneep/matplotlib/d73d20ad25d4341b1003fcfe04b17779a0e91207/lib/mpl_toolkits/mplot3d/axes3d.py
....
2021-04-09 17:10:45 (6.90 MB/s) - ‘axes3d.py’ saved [103843/103843]

$ diff -Naur axes3d.py /mingw64/lib/python3.8/site-packages/mpl_toolkits/mplot3d/axes3d.py
--- axes3d.py   2021-04-09 17:10:45.077761400 +0200
+++ /mingw64/lib/python3.8/site-packages/mpl_toolkits/mplot3d/axes3d.py 2021-04-09 17:09:50.724401000 +0200
@@ -1638,7 +1638,9 @@
                         colset.append(fcolors[rs][cs])
             # For the masking we have to loop because the polys may
             # not be regularly shaped
+            pmask = [ not( np.any(p[:, 3]) ) for p in polys ] # save first, for correspondingly masking colset
             polys = [p[:, :3] for p in polys if not np.any(p[:, 3])]
+            colset = [colset[ic] for ic, p in enumerate(pmask) if p] # apply same polys mask to colset

         # note that the striding causes some polygons to have more coordinates
         # than others

... will allow to also use facecolors, that would be "adjusted" to the masking of z; here is an example, that otherwise fails with this PR, with "ValueError: operands could not be broadcast together with shapes (22,1) (81,4)":

import numpy as np
import matplotlib.pyplot as plt

dimsize = 256 # 10
x = np.linspace(0,1.0,dimsize)
y = np.linspace(0,1.0,dimsize)
xg,yg = np.meshgrid(x,y)
z = 0.5*(xg**3 + yg**3)
zcol = np.reshape( np.reshape(z, (-1,1))*np.array([1,1,0.5]), (z.shape[0], z.shape[1], 3) )
zm = np.ma.masked_array(z, z<0.3)

fig = plt.figure(figsize=(8, 8))
ax1 = fig.add_subplot(221, projection='3d')
ax2 = fig.add_subplot(222)

surf = ax1.plot_surface(xg,yg,zm,
                 rstride=1, cstride=1, linewidth=0,
                 facecolors=zcol
                 )

ax1.set_zlim(0,1)
plt.colorbar(surf, ax=ax1, shrink=0.7, orientation='horizontal')

ax2.imshow(zm, cmap='inferno')

plt.show()

With the hack, this results with the following image:

jklymak · 2021-05-08T22:41:20Z

Is this still a viable PR? I'm not sure what still needs to be done here. @tomneep please feel free to ping us for reviews - things fall off the PR queue unfortunately. Thanks!

tomneep · 2021-05-13T16:36:44Z

Hi @jklymak, it was completely my fault, I got busy with other things. I'm happy to attempt to wrap this up. I've just pushed one change aiming to address one of the comments I received (and it simultaneously solves the problem raised by @sdbbs too). I'm a long way behind the master now, so lets see what the CI says.

tomneep · 2021-07-08T09:39:23Z

I had a quick look for related issues to this and #4941 is an interesting one. Before applying the changes in this branch, the example given there gives

and with this branch

I'd say that the fix gives what we'd want for masked data in 3D space, but the issue says they'd expect the masked data to be shown in red (the "bad" setting of the colormap). I'm not sure what the suggestion is here though, as if the area was present but red one would still be able to see the z values. I could do with some feedback on this.

jklymak · 2021-07-08T14:53:35Z

This is a conflict between what a mask means for the elevation, and what it means for the colormapping. Unfortunately those are the same thing for plot_surface. Personally I feel relatively strongly that the mask should apply to the elevation, in which case it is impossible to apply it to the colormap because there is nothing to color. How would we even know where to place the surface level? I guess we could look at the masked values and plot them if they make any sense, but then colormap them like they are bad data, but I really don't think that is going to be very reliable.

Its a bit awkward, but to get what is wanted in #4941 I think the user wants to colormap manually and pass in facecolor=C. I actually think we should provide a C argument here, and allow C=Z if it is not passed, and then C maps into the provided colormap.

jklymak · 2021-07-08T15:04:59Z

lib/mpl_toolkits/mplot3d/axes3d.py

@@ -1693,6 +1697,12 @@ def plot_surface(self, X, Y, Z, *args, norm=None, vmin=None,
                        cbook._array_perimeter(a[rs:rs_next+1, cs:cs_next+1])
                        for a in (X, Y, Z)
                    ]
+                    # If any of the perimeters are masked, then skip the


Can you test this?

I've added another test for this based on the example above

Is this behavior desirable? If everything but the perimeter of the polygon is masked, do we really want to draw the polygon? Some other choices would be:

Draw the polygon only if none of the contained elements are masked

Draw the polygon if less than half (or a user-configurable parameter of the contained elements are masked

Set the opacity of the polygon to be proportional to the maskedness

Attempt to precisely outline the masked regions

Whatever you decide on, I think it's important to describe the choice in the docstring.

It is a good question and I'm happy to have more input on this topic.

My line of thought is that as long as the perimeters of the polygons don't contain masked data, then it is ok to draw the polygon. If some data inside of the polygon is masked but is strided over, then it is not used to make the plot anyway so we aren't "revealing" masked data in the plot. I understand this might cause a bit of confusion due to the nature of the xstride/xcount settings.

Draw the polygon if less than half (or a user-configurable parameter of the contained elements are masked

and

Set the opacity of the polygon to be proportional to the maskedness

These seem like "advanced" use cases to me but happy to hear if others think differently. Though I still don't think anyone would want the perimeter of a polygon to contain masked data. As far as I can tell, no one has requested this kind of feature before.

Attempt to precisely outline the masked regions

Probably the ideal solution, if any one has suggestions how the can be accomplished then I'm happy to hear them!

I think I want input from @ianthomas23 about how he handled masked data for the contouring functions. I think it would make sense to be consistent with those rules.

Of course, the concepts don't perfectly overlap because the contouring algorithms don't have a concept of striding, but I am still interested in his input.

I don't believe striding does anything other than a simple decimation at the moment? If so I don't see why it would do something fancy for a mask. You could easily have the same complaint about a sharp spike that gets strided over. If folks want to properly decimate their data they can do so before passing to mpl.

I don't believe striding does anything other than a simple decimation at the moment?

It doesn't decimate the space fully, as the perimeter is still drawn at full precision. But looking closely, the colormap color is computed based only on the perimeter, so I guess the implemented behavior is consistent.

tomneep · 2021-07-08T15:12:51Z

Personally I feel relatively strongly that the mask should apply to the elevation, in which case it is impossible to apply it to the colormap because there is nothing to color. How would we even know where to place the surface level? I guess we could look at the masked values and plot them if they make any sense, but then colormap them like they are bad data, but I really don't think that is going to be very reliable.

Yes I completely agree with you. I'd say that what this PR does is the right thing to do and moves things in the right direction.

QuLogic

If you update the images and want to keep your other commits split apart when merged (instead of squash merged), please amend the commit with the image so that the old ones are not kept in the repo.

QuLogic · 2021-07-09T08:28:08Z

lib/mpl_toolkits/tests/test_mplot3d.py

+    Y = np.linspace(-1, 1)
+    X, Y = np.meshgrid(X, Y)
+    Z = X**2 + Y**2
+    Z[10:20, 10:20] = 0.0


The location of the hole is a bit difficult to see; can you move it somewhere move visible?

Thanks for the suggestion. I've updated the image with the hole in a more prominent place. I amended the commit though I have no preference on whether this is squash merged or not.

tomneep · 2021-07-16T11:32:38Z

Thanks for the reviews and discussion on this so far. During my investigations of this I think I've found a better way of addressing this, while fixing a few other open issues too. The approach is to allow NaNs in data passed to plot_surface and is implemented in #20646. Then dealing with masked arrays is just a case of adding

if isinstance(Z, np.ma.MaskedArray):
    Z = Z.astype(float).filled(np.nan)

somewhere in plot_surface. Getting the example from #4941 to work (really) nicely also requires a solution to #16470 too, which I have ready to go once I get some feedback on #20646. So in the end allowing NaNs would (eventually) close five issues, rather than the two this PR does. The final plots after all these changes also look nicer (in my opinion) as triangular polygons are included and not ignored like in this PR (compare the image below with the example at the top of the PR)

Of course, the big question is whether #20646 is viable? If so I think I can close this PR and move forwards with that

ianthomas23 · 2021-07-19T20:28:10Z

Thanks for the ping @WeatherGod.

To compare with contour/contourf handling of masked z, here is an example based on the Contour Corner Mask example (https://matplotlib.org/stable/gallery/images_contours_and_fields/contour_corner_mask.html#sphx-glr-gallery-images-contours-and-fields-contour-corner-mask-py):

import matplotlib.pyplot as plt
import numpy as np

x, y = np.meshgrid(np.arange(7), np.arange(10))
z = np.sin(0.5 * x) * np.cos(0.52 * y)

mask = np.zeros_like(z, dtype=bool)
mask[2, 3:5] = True
mask[3:5, 4] = True
mask[7, 2] = True
mask[5, 0] = True
mask[0, 6] = True
z = np.ma.array(z, mask=mask)

fig = plt.figure(figsize=(16, 4))
for i, corner_mask in enumerate([False, True]):
    ax = fig.add_subplot(1, 3, i+1)
    cs = ax.contourf(x, y, z, corner_mask=corner_mask)
    fig.colorbar(cs, ax=ax)
    ax.set_title('corner_mask = {0}'.format(corner_mask))
    ax.grid(c='k', ls='-', alpha=0.3)
    ax.plot(np.ma.array(x, mask=~mask), y, 'ro')

ax = fig.add_subplot(1, 3, 3, projection='3d')
ax.elev = 65
surf = ax.plot_surface(x, y, z, rstride=1, cstride=1, cmap='viridis')
fig.colorbar(surf, ax=ax)
        
plt.show()

Using the latest commit (3eace1e) from this PR the output is

showing that the plot_surface here is consistent with corner_mask=False contouring.

The approach of @tomneep's previous comment looks equivalent to corner_mask=True, and as this is the default for contouring I would prefer it. Of course, if only one of corner_mask=True and corner_mask=False is implemented, eventually someone will come along asking for the other as well!

jklymak · 2021-07-22T00:08:12Z

I'll move this to draft while you sort out #20646, but feel free to move back if you think they can be looked at independently.

QuLogic · 2021-08-12T05:25:08Z

Replaced by #20725, I believe.

dstansby added the topic: mplot3d label Jul 29, 2020

QuLogic reviewed Aug 1, 2020

View reviewed changes

lib/mpl_toolkits/mplot3d/axes3d.py Outdated Show resolved Hide resolved

QuLogic reviewed Aug 1, 2020

View reviewed changes

lib/mpl_toolkits/tests/test_mplot3d.py Show resolved Hide resolved

eric-wieser reviewed Aug 4, 2020

View reviewed changes

lib/mpl_toolkits/mplot3d/axes3d.py Outdated Show resolved Hide resolved

dstansby reviewed Sep 20, 2020

View reviewed changes

jklymak marked this pull request as draft May 12, 2021 20:16

jklymak added the status: orphaned PR label May 12, 2021

tomneep added 6 commits May 13, 2021 18:16

Add masked array support to 3d plot_surface (#487)

cf8647c

flake8 fix

682a767

Add simple test

f32c976

Review comments

b9dc0b1

A better way of skipping masked polygons

91d2f37

Fix test

296a388

tomneep marked this pull request as ready for review July 7, 2021 18:48

jklymak added status: needs review and removed status: orphaned PR labels Jul 7, 2021

jklymak mentioned this pull request Jul 8, 2021

Axes3d plot_surface not supporting masked arrays? #4941

Closed

jklymak reviewed Jul 8, 2021

View reviewed changes

QuLogic reviewed Jul 9, 2021

View reviewed changes

New test for masking when using non-optimal path

3eace1e

tomneep mentioned this pull request Jul 14, 2021

Handle NaN values in plot_surface zsort #20646

Closed

7 tasks

jklymak marked this pull request as draft July 22, 2021 00:08

tomneep mentioned this pull request Jul 23, 2021

Axes3D.plot_surface: Allow masked arrays and NaN values #20725

Merged

7 tasks

QuLogic closed this Aug 12, 2021

QuLogic removed the status: needs review label Aug 12, 2021

tomneep deleted the mplot3d_masked_surface branch August 19, 2021 08:42

Uh oh!

Mplot3d masked surface #18114

Mplot3d masked surface #18114

Uh oh!

Conversation

tomneep commented Jul 29, 2020

PR Summary

Demo

PR Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dstansby left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sdbbs commented Apr 9, 2021

Uh oh!

jklymak commented May 8, 2021

Uh oh!

tomneep commented May 13, 2021

Uh oh!

tomneep commented Jul 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jklymak commented Jul 8, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tomneep commented Jul 8, 2021

Uh oh!

QuLogic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tomneep commented Jul 16, 2021

Uh oh!

ianthomas23 commented Jul 19, 2021

Uh oh!

jklymak commented Jul 22, 2021

Uh oh!

QuLogic commented Aug 12, 2021

Uh oh!

Uh oh!

tomneep commented Jul 8, 2021 •

edited

Loading