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)":

importnumpyasnpimportmatplotlib.pyplotaspltdimsize=256# 10x=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:

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!

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.

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.

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 forplot_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 infacecolor=C. I actually think we should provide aC argument here, and allowC=Z if it is not passed, and then C maps into the provided colormap.

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.

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 toplot_surface and is implemented in#20646. Then dealing with masked arrays is just a case of adding

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

somewhere inplot_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

Thanks for the ping@WeatherGod.

To compare withcontour/contourf handling of maskedz, 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):

importmatplotlib.pyplotaspltimportnumpyasnpx,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]=Truemask[3:5,4]=Truemask[7,2]=Truemask[5,0]=Truemask[0,6]=Truez=np.ma.array(z,mask=mask)fig=plt.figure(figsize=(16,4))fori,corner_maskinenumerate([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=65surf=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 theplot_surface here is consistent withcorner_mask=False contouring.

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

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.

Replaced by#20725, I believe.