[ENH]: fill_between extended to 3D #28225
Conversation
e59859a to
6a13037
Compare
|
The white seams between the patches seems to be an instance of #1188, which I don't see a bug report for in 3D rendering. Is rather annoying but doesn't seem like there are easy fixes. |
6a13037 to
1e7c195
Compare
1e7c195 to
26cbf88
Compare
|
Note that I decided to not implement the "mode 2" idea in https://github.com/artmenlope/matplotlib-fill_between-in-3D/blob/master/FillBetween3d.py, which concatenates all the points and plots them as one polygon. I think the only benefit is getting rid of the white seams between the patches in the filled flat curves example, but it is degenerate in the general case, and this use case can be handled by users defining their own polygons such as in the new For example, Mode 1 / Mode 2: |
|
Here's the new plot types thumbnail showing a double helix:
|
779b177 to
76eff4a
Compare
12ba10d to
3febc1a
Compare
There was a problem hiding this comment.
Requesting changes just so someone doesn't merge without my comment being addressed.
I think "mode 1" vs "mode 2" should be investigated here, and I'd like to see the new test case with the "mode 2" version as well to see if that applies the edge color to only the rim/boundary of the single fill_between region. Naively thinking about it that would be my preference for how to draw these, but I also haven't given it a ton of thought.
3febc1a to
d1607af
Compare
d1607af to
de0d319
Compare
|
@greglucas I've added the "mode 2" |
9c89b67 to
ad23551
Compare
|
Re-requesting @timhoffm's review because it is quite a bit different now with a new keyword argument now and he might have opinions on that from the API standpoint. |
ad23551 to
659ec35
Compare
|
Changed the |
|
We could try to get smart about |
|
The more I think about it, the more I like the optimization to map |
There was a problem hiding this comment.
We may also want to think about shade. It's deactivated by default for Poly3dCollection, but activated by default for cases where the drawing is a geometrical object, e.g. in plot_surface() if it's a single color.
Your quad example (at least without edgecolor) could benefit from shading. Without having this thought through completely 'quad' likely wants it on by default, 'polygon' possibly wants it of - there's no shading within the single polygon, but OTOH one could make the polygon example single color and use depth shading to create visual distinction.
Please think about what's the best solution. The default may depend on mode. In any case, I think it's valuable to mention it in the docstring.
lib/mpl_toolkits/mplot3d/axes3d.py
Outdated
| had_data = self.has_data() | ||
| x1, y1, z1, x2, y2, z2 = cbook._broadcast_with_masks(x1, y1, z1, x2, y2, z2) | ||
| if mode == 'auto': | ||
| if np.all(x1 == x2) or np.all(y1 == y2) or np.all(z1 == z2): |
There was a problem hiding this comment.
The more I think about it, the more I like the optimization to map auto to polygon when all the points are on an x, y, or z plane. It doesn't cover all the cases of points on an arbitrary plane, but it will cover what I think will be the most common ones, and the user can always choose the mode directly.
How hard would it be to generalize to "if all points lie in a plane"? You could construct a plane from the first 3 points and then calculate a dot product for all following to check distance to that plane and it wouldn't be too bad.
There was a problem hiding this comment.
@greglucas I added art3d._all_points_on_plane() to check for this, it's a little involved to catch the edge cases - what do you think?
There was a problem hiding this comment.
Sorry for sending you down the rabbit hole 😂 . I think it is great! and I really appreciate that you checked the edge cases. Maybe it is a little involved, but seems worth it to me to generalize the statement to any plane. It is only a one-time performance hit when creating the collection initially and it is all numpy-vectorized so it isn't that terrible.
bf8cde7 to
6750726
Compare
fill_between in plot types Apply suggestions from code review Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com> fill_between single_polygon flag 3D fill_between auto mode maps to polygon when all points lie on a x, y, or z plane 3D fill_between auto mode maps to polygon when all points lie on a x, y, or z plane Code review comments fill_between 3d shading fill_between 3d shading
b88c8a1 to
3ef2340
Compare
|
@timhoffm I do like the addition of the Here's the top example with shading and no transparency: |
731b26f to
d84eef1
Compare
lib/mpl_toolkits/mplot3d/axes3d.py
Outdated
| had_data = self.has_data() | ||
| x1, y1, z1, x2, y2, z2 = cbook._broadcast_with_masks(x1, y1, z1, x2, y2, z2) | ||
| if mode == 'auto': | ||
| if np.all(x1 == x2) or np.all(y1 == y2) or np.all(z1 == z2): |
There was a problem hiding this comment.
Sorry for sending you down the rabbit hole 😂 . I think it is great! and I really appreciate that you checked the edge cases. Maybe it is a little involved, but seems worth it to me to generalize the statement to any plane. It is only a one-time performance hit when creating the collection initially and it is all numpy-vectorized so it isn't that terrible.
d84eef1 to
755d7ab
Compare
755d7ab to
dd05f32
Compare
|
CI failures are unrelated |
|
Thanks @scottshambaugh. Great addition! |
PR summary
Closes #28142
This extends the 2D
fill_betweenmethod to 3D plots, and IMO works quite nicely. I’m finding it really powerful to generate surfaces easily. Inspired by @artmenlope's solution here: https://github.com/artmenlope/matplotlib-fill_between-in-3D/blob/master/FillBetween3d.pyHere's the what's new image and
fillbetween3dgallery example:Note that I moved the
poly3dexample https://matplotlib.org/3.9.0/gallery/mplot3d/polys3d.html to a new examplefillunder3d, to use this new method. Then I made a new example forpoly3dsince adding 3D collections wasn't shown anywhere else afaik.New
fillunder3dexample:New
poly3dexample:PR checklist