-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
DOC Clean up on about half the Mplot3d examples #6303
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DOC Clean up on about half the Mplot3d examples #6303
Conversation
attn @WeatherGod |
The failures on Travis are legit, some pep8 (forgot to pylint the last couple files I worked on) and apparently my polys3d_demo.py throws an error on the Travis environment that isn't thrown on my computer. I'll sort these out tonight or tomorrow. |
d86994e
to
53b088c
Compare
The errors I mentioned are fixed now. By the way, I haven't changed any file names yet but as I understand it the '_demo' should be dropped from all names and files that are only differentiated by numbers should get more descriptive names. I don't have any good ideas for renaming the five contour examples, any suggestions? |
cset = ax.contour(X, Y, Z, zdir='x', offset=-40, cmap=cm.coolwarm) | ||
cset = ax.contour(X, Y, Z, zdir='y', offset=40, cmap=cm.coolwarm) | ||
cset = ax.contour(X, Y, Z, zdir='z', offset=-100, cmap=cm.coolwarm) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably should double-check that this doesn't make an graphing artifacts worse. I am pretty sure I put this one first for a reason...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean that it's better to plot the z one first because it's on the 'bottom' from the perspective of the default angle, or something else? And if so, do you think that choice warrants a comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In some cases, yes. It is what I call the "Escher Effect". When the 3d bounding box of an artist (as oriented in the viewer's coordinate system) intersects with the 3d bounding box of another artist, the z-order trick that mplot3d uses becomes insufficient. Plotting some things before others can sometimes help, but not always. I don't know how much you'd want to go into it in these examples. I do cover it a bit in mplot3d's FAQ.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I have seen that kind of Escher effect at least once... when I was working on hist3d_demo I toyed with bar sizes and noticed that things got really weird when multiple bars overlapped. I'm good with changing the order back and not writing a comment, if users try another ordering and encounter trouble then it's likely that either they'll guess how to fix it or they'll be able to google it and find out.
…urface, lines3d_demo, offset_demo
… instead of zeroing out the first and last elements of the random data
53b088c
to
ab0d48c
Compare
All discussed changes are incorporated, I amended them into their corresponding commits. |
Thanks for your efforts. Cleanups are always welcomed! |
My goal is to make it easy to understand what the code is doing, so people flipping through examples can quickly judge if an example has something they can use, and so people borrowing a bit of code will understand how to modify it responsibly, so to speak.
There are guidelines on MEP12 and comments on issue #6221 about what examples should strive to do. I may not have ticked every single box mentioned in those places, but I've made the formatting a lot more consistent, added a lot of docstrings and comments, and made other mostly cosmetic changes. Let me know if there are any particular style points you'd like to see done differently.
I made one nontrivial change, to the behaviour of polys3d_demo. I changed it so that it forms the bottom facet of each polygon by creating two new vertices rather than overwriting two of the y data points. I thought it was better to assume that a user might want to do this using some data of theirs which shouldn't be overwritten.