| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
Well, it is at least possible to import STL-files and add them to a Poly3DCollection with shading, but some things can be improved, like that it is required to provide a color the way the code is now. |
Sorry, something went wrong.
| ------------------------------------- | ||
|
|
||
| It is now possible to shade a `.Poly3DCollection`. This is useful if the | ||
| polygons are obtained from e.g. a 3D model. |
There was a problem hiding this comment.
Slightly unclear cause it's missing the general thing a 3D model is an example of.
Sorry, something went wrong.
There was a problem hiding this comment.
I agree that this formulation is maybe not ideal. I was originally going to discuss things like STL-files, but since that is not the only way to represent a 3D model I kept it simple.
An option is to generate a static example loading an STL model (as we need additional libraries).
Sorry, something went wrong.
There was a problem hiding this comment.
In what cases besides a 3D model would shading be useful?
Sorry, something went wrong.
There was a problem hiding this comment.
I guess that one can have some other type of 3D polygons that are not necessarily considered to be a "3D model", although in some sense all 3D polygons could be considered a "3D model". One example may be voxels? Basically something that is not representing a physical object, but a visualization of something that happens to be 3D Polygons.
But I am open for suggestions.
Sorry, something went wrong.
Sorry, something went wrong.
|
Shading of edgecolor is a bit dodgy by the way since it is not that straightforward to determine the normal of a line... (Not sure how it is actually done.) |
Sorry, something went wrong.
There was a problem hiding this comment.
I'm nitpicking language you didn't write in the first place, so I can totally just open my own pr for that. I think the refactor into one place is great.
Sorry, something went wrong.
| if facecolors is None and edgecolors in None: | ||
| _api.warn_external( | ||
| "You must provide facecolors, edgecolors, or both for " | ||
| "shade to work as expected.") |
There was a problem hiding this comment.
| "shade to work as expected.") | |
| "shade to work.") |
Should we error out instead of warn? It seems to be incorrect usage, and I don’t see a reason why a user might want to set shade without having the results shaded.
Sorry, something went wrong.
There was a problem hiding this comment.
The idea was that one can, sort of, expect the default colors to be used. However, those are currently not assigned until after the shading is applied. So I thought that a warning made more sense since this is something that can be changed in the future.
Sorry, something went wrong.
There was a problem hiding this comment.
My point is
Sorry, something went wrong.
There was a problem hiding this comment.
Modulo a minor docstring suggestion.
Sorry, something went wrong.
| .. versionadded:: 3.7 | ||
|
|
||
| .. note:: | ||
| *facecolors* and/or *edgecolors* must be provided for shading | ||
| to work. |
There was a problem hiding this comment.
I think a note is more heavy than needed. We usually only state deprendencies between parameters in plain text:
| .. versionadded:: 3.7 | |
| .. note:: | |
| *facecolors* and/or *edgecolors* must be provided for shading | |
| to work. | |
| When activating *shade*, *facecolors* and/or *edgecolors* must be | |
| provided. | |
| .. versionadded:: 3.7 |
Sorry, something went wrong.
There was a problem hiding this comment.
Sure. Again this comes from the fact that ideally it should work without providing it, so I thought it was worth highlighting. But as it now errors as well, it is hard to miss it.
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
PR Summary
Fixes #19509.
Refactors shading of Poly3DCollection and moves it to the constructor of the collection rather than in every individual plotting method, Can still be done anywhere though.
PR Checklist
Tests and Styling
Documentation