| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
There was a problem hiding this comment.
Thank you for opening your first PR into Matplotlib!
If you have not heard from us in a while, please feel free to ping @matplotlib/developers or anyone who has commented on the PR. Most of our reviewers are volunteers and sometimes things fall through the cracks.
You can also join us on gitter for real-time discussion.
For details on testing, writing docs, and our review process, please see the developer guide
We strive to be a welcoming and open project. Please follow our Code of Conduct.
Sorry, something went wrong.
|
Thanks for the PR! I unfortunately don't have concrete suggestions but I'm wondering if some of these examples can be smaller/more straightforward? The goal of this gallery isn't necessarily pretty pictures, just examples that clearly show what data goes into what plots & what plots are available. |
Sorry, something went wrong.
|
Thanks for the feedback. I have curated a more straightforward set of examples in matplotlib/plot_types/3D. Sorry about all of the commits, this is my first pull request and I'm still getting used to Git. If I need to rebase or do anything else, please let me know. I will wait for feedback before making any new commits or merges. |
Sorry, something went wrong.
|
It looks like your code got muddled in w/ the rebase. Git is a pain. I'm gonna suggest cherry picking b/c it's what I just did to clean a branch, but there may be an easier way to clean your branch. Also feel free to drop by our incubator channel w/ questions!
commit 910ed53147058d02f43bd02865e7420be08e4969 (HEAD ->'add-3d-plots-to-gallery, origin/'add-3d-plots-to-gallery)
3d example code
commit cada8fb565b1e1cfeb076f1f6ab791b1640d387d (upstream/main, main)
Merge: 363713442b da3790fc4e
|
Sorry, something went wrong.
|
These examples need to become more in line with the rest of plot types (take the others as example):
|
Sorry, something went wrong.
There was a problem hiding this comment.
This is loads better! I'm a little concerned that the voxels example is still trying to do a drop too much.
The big things this needs is consistent figure creation and please follow the plot gallery style, which is that the examples have a title and then the next line is a link to the function being illustrated in that example
https://github.com/matplotlib/matplotlib/blob/main/plot_types/basic/scatter_plot.py
Sorry, something went wrong.
|
Almost there! The gallery examples follow a # make data, # plot format which you've done in trisurf, voxels; please amend scatter, surface, and wireframe to match. Also, the failed doc tests means I can't see how the new style is being applied to the gallery entries. I think sphinx is mad b/c you're using the same names as the gallery example:
so please change the names - appending simple should be fine. |
Sorry, something went wrong.
|
Thanks again for feedback, everyone. Where was that warning message about duplicated file names? I didn't see it under "Details" for "View the built docs" or "ci/circleci: docs-python38", but my browser was having issues showing the previous circleci.com page. Edit: I see the warnings on circleci now. |
Sorry, something went wrong.
|
This is looking good. You have a build failure however.
Thanks! |
Sorry, something went wrong.
|
One way to remove the tick labels is by adding ax.set_xticklabels([]), ax.set_yticklabels([]), and ax.set_zticklabels([]). Is this acceptable? Or is there a cleaner, preferred way to remove all tick labels? Maybe through styling? |
Sorry, something went wrong.
|
the examples all use ax.set to set stuff like ticks and you may need to edit the styles at https://github.com/matplotlib/matplotlib/tree/main/lib/matplotlib/mpl-data/stylelib to add in 3D configs |
Sorry, something went wrong.
There was a problem hiding this comment.
Yay, thanks for putting up w/ all the tweaks!
I think it's good enough to merge, but if you have bandwidth, I'd be interested in seeing some shades of blue color variation on the surfaces/wireframes and the voxels colored two colors.
Sorry, something went wrong.
|
For the surfaces, I agree that a color gradient could increase visual clarity. I think these changes work well while adding minimal complexity. Regarding the voxels, the color-related code was removed from the original gallery example. Is reintroducing distinct colors worth increasing the Plot Types code complexity? For the wireframe, after reading through the docs and doing some testing, it is unclear if a z-coordinate-dependent color gradient can be added to each plotted line. And the cmap keyword, e.g. cmap=cm.viridis, does not change the wireframe appearance. |
Sorry, something went wrong.
|
Yup you're right, wireframe doesn't support colormapping. Sorry about that! And I don't have strong opinions on voxels, so will agree w/ you to keep it on the simple less pretty side. |
Sorry, something went wrong.
| 3D | ||
| -- | ||
|
|
||
| 3D plots using the mpl_toolkits library. |
There was a problem hiding this comment.
Tiny nit but can you crosslink this to the https://matplotlib.org/devdocs/api/toolkits/mplot3d.html API docs?
Sorry, something went wrong.
|
Congrats 🥳 on your first PR into matplotlib, thank you so much for your patience with our feedback 😅, and we hope to see you again! |
Sorry, something went wrong.
…108-on-v3.6.x Backport PR #24108 on branch v3.6.x (Add 3D plots to plot_types doc page)
| Back | FazBrowse Home | New Git URL |
PR Summary
Add assorted 3D plot examples from the general gallery (https://matplotlib.org/devdocs/gallery/mplot3d/index.html) to the plot_types gallery (https://matplotlib.org/devdocs/plot_types/index.html) to increase visibility of Matplotlib's 3D capabilities.
Issue link: #23900
PR Checklist
Tests and Styling
Documentation