Tidying up and tweaking mplot3d examples [MEP12] #6690

TrishGillett · 2016-07-05T06:44:12Z

This PR is the follow up to #6303 and covers clean up work for all the mplot3d examples not covered in that one. It also includes touch ups to an example cleaned up previously, 2dcollections3d_demo. A lot of my changes are docstring and comment additions, but I've also done some rearranging and refactoring in hopes of improving clarity. I'll make line comments on some of the points of interest.

TrishGillett · 2016-07-05T06:49:04Z

examples/mplot3d/rotate_axes3d_demo.py

 for angle in range(0, 360):
    ax.view_init(30, angle)
    plt.draw()
+    plt.pause(.001)


On my computer, this line was needed to make the example viewable. I got the idea for it when I did the wire3d_animation_demo example, but I'm not sure this is the correct practice since in both examples this line seems to trigger a deprecation warning:

/Users/trish/anaconda/lib/python2.7/site-packages/matplotlib-2.0.0b1+1720.ga8ad349.dirty-py2.7-macosx-10.5-x86_64.egg/matplotlib/backend_bases.py:2444: MatplotlibDeprecationWarning: Using default event loop until function specific to this GUI is implemented warnings.warn(str, mplDeprecation)

Let me know if there's something else I should do with this instead.

Which backend are you using?

Dumb question: how do I tell?

import matplotlib; print(matplotlib.get_backend())

Thanks! Qt4Agg.

If I'm going to make an issue for this, which part is the issue? That nothing displays for me without the use of plt.pause(), or that the use of plt.pause() triggers that warning? I'm not really clear on whether plt.draw() is supposed to be sufficient in an example like this.

This may be because plt.draw is now lazy and won't draw until idle?

@QuLogic You are correct. plt.pause() spins the GUI event loop so the update gets processed.

I don't think we need to worry about the warning in this PR, it is a know issue.

TrishGillett · 2016-07-05T14:32:35Z

examples/mplot3d/wire3d_animation_demo.py


+# Set the z axis limits so they aren't recalculated each frame.
+ax.set_zlim(-1, 1)


The z axis scale was changing as the rotation progressed, so this seemed like a nice addition.

That shouldn't happen. Makes me wonder if it is a bug with the style changes for 2.0.

I'm doing a git bisect to try and nail down when this happened.

Using this example as a test script, I see the same behaviour with versions 1.5.2, 1.5.1, 1.5.0, 1.4.0, 1.2.0, and 1.0.0. I haven't found a commit where this behaviour doesn't happen.

ok, how about this issue and the other one with regards to "plt.pause()"
get filed as issues? Let's take out those two changes for now and keep it
focused on just MEP12. We'll have to investigate those two issues further
to see if there is something deeper going on.

On Tue, Jul 5, 2016 at 11:14 AM, Trish Gillett-Kawamoto <
notifications@github.com> wrote:

In examples/mplot3d/wire3d_animation_demo.py
#6690 (comment):

+# Set the z axis limits so they aren't recalculated each frame.
+ax.set_zlim(-1, 1)

Using this example as a test script, I see the same behaviour with
versions 1.5.2, 1.5.1, 1.5.0, 1.4.0, 1.2.0, and 1.0.0. I haven't found a
commit where this behaviour doesn't happen.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/matplotlib/matplotlib/pull/6690/files/6a5def2d4d237346f47e4e9da592f05058b1a9f9#r69580992,
or mute the thread
https://github.com/notifications/unsubscribe/AARy-L05A6cqDxNM8ebAlLR7HrjuixW9ks5qSnTcgaJpZM4JE0ZQ
.

I'm just realizing I was thinking of the rotate_axes3d example when I wrote the note saying the z limits changed 'as the rotation progressed'. Sorry for that confusion. In fact it's the data that's changing in this example, not the rotation. As the largest z value contracts from say 1 to 0.6, the z limits are self-adjusting so that the extremes of the surface are always using the full z range. In that updated context, would you say that the behaviour is as expected or does something still sound fishy?

@TrishGillett That behavior is expected given how this code works.

tacaswell · 2016-10-11T03:46:43Z

Restarted the failing tests (which are almost certainly un-related).

NelleV · 2016-10-11T05:26:34Z

I have also restarted the failing test as it seemed unrelated.

Thanks for the patch @TrishGillett ! The examples are much better.
If you ever work on the examples again, do you mind having the following structure of docstrings:

"""
title

description
"""

It'll make our life easier when we move to sphinx-gallery, as this is the required format for examples docstring and sphinx-gallery.

TrishGillett · 2016-10-11T05:33:42Z

Thanks for the approvals! Duly noted about the docstring structure, I'll check it too from now on. :)

NelleV · 2016-10-11T05:42:30Z

The travis failure is unrelated, but I have no clue what's going on: it seems a font file is missing.
@tacaswell any idea?

TrishGillett · 2016-10-11T12:03:06Z

Any chance a rebase might help?

tacaswell · 2016-10-11T13:56:39Z

A rebase might help. Not sure what is going wrong with that 2.7 test....

TrishGillett · 2016-10-12T14:15:55Z

I'll do a rebase tonight or tomorrow. While I'm at it, I can change the docstrings like @NelleV mentioned. Is the idea that the 'title' should match the filename (ex. 2dcollections3d_demo.py => '2D Collections 3D Demo')? If not, can you give me an example of what you're going for?

NelleV · 2016-10-13T16:30:48Z

First, don't feel obliged to do this. Your work on this PR is already a huge improvements of our examples.

In short, a blocker for one of my pull request is the migration of our gallery to sphinx-gallery. Unfortunately, sphinx gallery requires that docstring be of the format "title + description". The title is used both in the gallery as a title to each vignette, and as the title of the docstring rendered for each example.
Here are some examples of titles: http://sphinx-gallery.readthedocs.io/en/latest/

I'll go other some of the plots to make suggestions.

NelleV · 2016-10-13T16:32:32Z

examples/mplot3d/surface3d_demo.py

@@ -1,23 +1,38 @@
+'''


A title on that PR could be "3D surfaces"

TrishGillett · 2016-10-13T17:08:04Z

It's no problem to add the titles, my goal when I do MEP12 PRs is to try and make the examples 'compliant' with all the standards we're shooting for, so I'd rather do it as part of that process than leave it for someone else to come back and do later. Maybe you should modify the 'Add introductory sentence or paragraph in the main docstring' point in the guidelines here to reflect what you need so some MEP12ers can help you chip away at the problem. :)

NelleV · 2016-10-13T17:09:45Z

That's on my todo list :)

…catter3d_demo, subplot3d_demo, and text3d_demo: Comments, docstrings and tweaks. [MEP12]

…aks, and refactoring. [MEP12]

…, and refactoring. [MEP12]

…nd refactoring. Notably, viewing angles on tricontour examples are tweaked and the graphs of trisurf3d_demo2 are made into subplots. [MEP12]

TrishGillett · 2016-10-16T17:56:17Z

Added example titles and rebased on master. Travis is running clean now but Appveyor has some complaints. Can someone in the know check if they're related or not?

efiring · 2016-10-16T18:41:06Z

Appveyor failures were unrelated.

Tidying up and tweaking mplot3d examples [MEP12] Conflicts: examples/mplot3d/2dcollections3d_demo.py Conflict resolved by taking the version from #6690

efiring · 2016-10-16T18:55:17Z

backported to v2.x as a869cc0

efiring · 2016-10-16T19:10:52Z

Thank you, @TrishGillett.

mdboom added the status: needs review label Jul 5, 2016

TrishGillett reviewed Jul 5, 2016
View reviewed changes

afvincent added topic: mplot3d Documentation labels Jul 5, 2016

tacaswell added this to the 2.0.1 (next bug fix release) milestone Jul 5, 2016

TrishGillett reviewed Jul 5, 2016
View reviewed changes

TrishGillett force-pushed the mplot3d-examples-MEP12-b branch from 6a5def2 to bced32d Compare July 5, 2016 17:51

QuLogic added the MEP: MEP12 gallery and examples improvements label Jul 5, 2016

tacaswell approved these changes Oct 11, 2016

View reviewed changes

tacaswell changed the title ~~Tidying up and tweaking mplot3d examples [MEP12]~~ [MRG+1] Tidying up and tweaking mplot3d examples [MEP12] Oct 11, 2016

NelleV approved these changes Oct 11, 2016

View reviewed changes

NelleV changed the title ~~[MRG+1] Tidying up and tweaking mplot3d examples [MEP12]~~ [MRG+2] Tidying up and tweaking mplot3d examples [MEP12] Oct 11, 2016

NelleV reviewed Oct 13, 2016

View reviewed changes

examples/mplot3d/surface3d_demo.py

@@ -1,23 +1,38 @@

'''

Copy link

Member

NelleV Oct 13, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A title on that PR could be "3D surfaces"

NelleV mentioned this pull request Oct 16, 2016

MEP12 - added sphinx-gallery docstrings #7277

Merged

TrishGillett added 3 commits October 16, 2016 11:19

DOC Change ax.set_xlim3d to ax.set_xlim, etc.

885a82b

DOC Cleaning up mplot3d examples quiver3d_demo, rotate_axes3d_demo, s…

890e880

…catter3d_demo, subplot3d_demo, and text3d_demo: Comments, docstrings and tweaks. [MEP12]

DOC Cleaning up mplot3d/surface3d* examples: comments/docstrings, twe…

13bb609

…aks, and refactoring. [MEP12]

TrishGillett added 2 commits October 16, 2016 11:19

DOC Cleaning up mplot3d/wire3d* examples: comments/docstrings, tweaks…

3243b51

…, and refactoring. [MEP12]

DOC Cleaning up mplot3d/tri* examples: comments/docstrings, tweaks, a…

bfbf0ca

…nd refactoring. Notably, viewing angles on tricontour examples are tweaked and the graphs of trisurf3d_demo2 are made into subplots. [MEP12]

TrishGillett force-pushed the mplot3d-examples-MEP12-b branch from bced32d to f14e90d Compare October 16, 2016 15:21

Add example titles in sphinx-gallery style.

d47dcce

TrishGillett force-pushed the mplot3d-examples-MEP12-b branch from f14e90d to d47dcce Compare October 16, 2016 16:00

efiring changed the title ~~[MRG+2] Tidying up and tweaking mplot3d examples [MEP12]~~ Tidying up and tweaking mplot3d examples [MEP12] Oct 16, 2016

efiring merged commit 0612ed5 into matplotlib:master Oct 16, 2016

efiring removed the status: needs review label Oct 16, 2016

QuLogic modified the milestones: 2.0.1 (next bug fix release), 2.0 (style change major release) Dec 7, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tidying up and tweaking mplot3d examples [MEP12] #6690

Tidying up and tweaking mplot3d examples [MEP12] #6690

TrishGillett commented Jul 5, 2016

TrishGillett Jul 5, 2016

WeatherGod Jul 5, 2016

TrishGillett Jul 5, 2016

WeatherGod Jul 5, 2016

TrishGillett Jul 5, 2016

TrishGillett Jul 5, 2016

QuLogic Oct 10, 2016

tacaswell Oct 11, 2016

TrishGillett Jul 5, 2016

WeatherGod Jul 5, 2016

TrishGillett Jul 5, 2016

TrishGillett Jul 5, 2016

WeatherGod Jul 5, 2016

TrishGillett Jul 5, 2016

tacaswell Oct 11, 2016

tacaswell commented Oct 11, 2016

NelleV commented Oct 11, 2016

TrishGillett commented Oct 11, 2016 •

edited

NelleV commented Oct 11, 2016

TrishGillett commented Oct 11, 2016

tacaswell commented Oct 11, 2016

TrishGillett commented Oct 12, 2016

NelleV commented Oct 13, 2016

NelleV Oct 13, 2016

TrishGillett commented Oct 13, 2016

NelleV commented Oct 13, 2016

TrishGillett commented Oct 16, 2016

efiring commented Oct 16, 2016

efiring commented Oct 16, 2016

efiring commented Oct 16, 2016


		# Set the z axis limits so they aren't recalculated each frame.
		ax.set_zlim(-1, 1)

Tidying up and tweaking mplot3d examples [MEP12] #6690

Tidying up and tweaking mplot3d examples [MEP12] #6690

Conversation

TrishGillett commented Jul 5, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tacaswell commented Oct 11, 2016

NelleV commented Oct 11, 2016

TrishGillett commented Oct 11, 2016 • edited

NelleV commented Oct 11, 2016

TrishGillett commented Oct 11, 2016

tacaswell commented Oct 11, 2016

TrishGillett commented Oct 12, 2016

NelleV commented Oct 13, 2016

Choose a reason for hiding this comment

TrishGillett commented Oct 13, 2016

NelleV commented Oct 13, 2016

TrishGillett commented Oct 16, 2016

efiring commented Oct 16, 2016

efiring commented Oct 16, 2016

efiring commented Oct 16, 2016

TrishGillett commented Oct 11, 2016 •

edited