I'm slightly worried that we are throwing away data and not storing the original inputs in thePolyCollection. Do we have precedent for that?

I don't know, hopefully someone else does. The one thought I had was to see what is done incontour/contourf. Here it looks like the masked data are not saved in thecollections (but perhaps I am missing something)? Would this be an equivalent case?

importmatplotlib.pyplotaspltimportnumpyasnpx,y=np.mgrid[-10:10.1:2,-10:10.1:2]z=np.ma.masked_less(x**2+y**2,50)fig,ax=plt.subplots(figsize=(6,6))contours=ax.contourf(x,y,z)segs=np.concatenate([np.concatenate(s)forsincontours.allsegsifs]).Tpaths=np.concatenate(    [i.verticesforpathsincontours.collectionsforiinpaths.get_paths()]).Tax.plot(segs[0],segs[1],"rs")ax.plot(paths[0],paths[1],".b")

gives

The user threw the data away when they NaN-ed or masked it. I don't think we are going to offer a way to unmask it after the surface is made, so I don't think there is harm in removing it. But maybe I'm missing a subtlety in the data->polygon pipeline. I assume if the user changes x, y, Z in the data then the polygons are regenerated anyway?

Sorry@jklymak I don't understand your question. What kind of changing of x, y and Z do you have in mind?

viaset_data or whatever the 3-d equivalent is...

I'm not sure what the equivalent would be. You could get the paths withget_paths and perhaps modify those, but it would be tricky. Maybe there is some easy method that I am missing.

The original x, y and z don't get passed to thePoly3DCollection. I suppose I could imagine a new collection that computed the polygons from the x, y and z and would recalculate the polygons ifset_data was called, but that isn't how it currently works. Is that what you had in mind?

Sorry if I'm taking you off on a tangent - many of our methods save a representation of the original data. If this has no such representation, then there really is no harm done in throwing out masked data because the user can't modify it post-facto anyways.

ThePoly3DCollection is made directly from the vertices of the polygons and doesn't see the original data. It isn't just the masking because there are thestride/count options inplot_surface that (can) also throw data away before the polygons are made. So Ithink we are ok on that front?

@eric-wieser@WeatherGod@ianthomas23 do any of you have time to comment onthis solution to the masking problem? It seems reasonable tome but I don't know if I've ever usedplot_surface in matplotlib ;-) Thanks!

Can you add a test showing how this behaves with striding? For instance, if a facet with stride=4 has all the. entries masked in this pattern, does matplotlib draw a diamond or two triangles?

++...+.............+...++

@eric-wieser Here is the example you request (for before and after this PR). It is a bit hard to show here but from the side angle (third plot) it doesn't look right, neither before and after this PR. Is this what you wanted to see?

Example without PR

Example with PR

Code for plots

importmatplotlib.pyplotaspltimportnumpyasnpx,y=np.mgrid[-2:2.1:1,-2:2.1:1]z=np.ma.masked_less(x*y,2)fig, (ax1,ax2,ax3)=plt.subplots(figsize=(9,3),ncols=3,subplot_kw={"projection":"3d"})ax1.plot_surface(x,y,z)ax1.set_title("Default strides")ax1.view_init(60,-45)surf=ax2.plot_surface(x,y,z,rstride=4,cstride=4)ax2.set_title("r/cstride = 4")ax2.view_init(60,-45)surf=ax3.plot_surface(x,y,z,rstride=4,cstride=4)ax3.set_title("r/cstride = 4")ax3.view_init(2,30)

Thanks, that does show what I was expecting to see. I don't think the side view is interesting, but I think the middle plot might be worth adding a test for in case someone decides they want different behavior later. Usingnp.mgrid[-6:6.1:1, -6:6.1:1] but leaving the stride at 4 would probably produce a slightly more insightful plot for deciding whether the current behavior is sensible.

Thanks, I've added a test with the range you suggest. Indeed the plot looks a bit more interesting than my previous comment, with a double arrowhead/bowtie arrangement. Does this look ok to you?

Yes, that looks like a good test case to include, thanks! My question (possibly for a future PR) would be whether it makes sense to split that middle polygon in two, into two triangles - but it's not immediately clear to me how such an approach would generalize.

Yes I had the same thought after the first example (whether the polygon could be split into pieces to help the side view). I'm not sure how simple it is, so I think as you say it might be a question for a future PR.

I'm not sure we should be worried about striding effects - if you decimate your data you should expect aliasing, and if you decimate a mask it is luck of the draw where on the mask your stride will fall. If you need more control, it really isn't hard to stride your own data.

I'd dispute that the claim that it's easy to stride your own data - this isn't just simple striding that takes 1/n² of the elements, it's preserving full resolution along the gridlines and discarding data in the cells, taking (2n-1)/n² of the elements. That means there's no way to represent the striding in a numpy array.

That's not to say you're wrong in saying that the weirdness in the plot above is unimportant - I think it's fine to merge with the current behavior, and was only pushing for a test to ensure we were aware of the quirk.

this isn't just simple striding that takes 1/n² of the elements, it's preserving full resolution along the gridlines and discarding data in the cells, taking (2n-1)/n² of the elements. That means there's no way to represent the striding in a numpy array.

probably off topic, but that is confusing. The docs sayrstride, cstride : Downsampling stride in each direction. Given that, I'm not sure what "full resolution along the gridlines and discarding data in the cells" means.

this isn't just simple striding that takes 1/n² of the elements, it's preserving full resolution along the gridlines and discarding data in the cells, taking (2n-1)/n² of the elements. That means there's no way to represent the striding in a numpy array.

probably off topic, but that is confusing. The docs sayrstride, cstride : Downsampling stride in each direction. Given that, I'm not sure what "full resolution along the gridlines and discarding data in the cells" means.

I mean that rstride=cstride=3 samples the input data as

########  #  ##  #  #########  #  ##  #  ########

Not

#  #  ##  #  ##  #  #

(Where every character is an entry in the input data array, and every # is a point that gets plotted)

So is the problem that the docs are wrong or my understanding of the term "stride" is wrong?

The docs are using "stride" in a vague sense, so they're not wrong per se, but certainly easy to misread. The reading I have of them that is consistent with the implementation is "Split the surface into row and column gridlines spacedrstride andcstride entries apart, but draw the perimeters of the resulting grid squares without downsampling". I suppose this phrasing in the docs is actively misleading:

If the input data is larger, it will be downsampled (by slicing) to these numbers of points.

I think the current behavior of the striding (ignoring this PR) is the right behavior, but the docs don't describe it well.

It appears this is good to go, and the striding discussion is orthogonal, so I'm going to merge this.