Skip to content

Merged streamline examples#8336

Merged
dstansby merged 4 commits into
matplotlib:masterfrom
patniharshit:mergedStreamline
Apr 12, 2017
Merged

Merged streamline examples#8336
dstansby merged 4 commits into
matplotlib:masterfrom
patniharshit:mergedStreamline

Conversation

@patniharshit

Copy link
Copy Markdown
Contributor

Based on discussion in #8155 a new pull request combining work with #8082.
Refs #7956

NelleV
NelleV previously requested changes Mar 19, 2017

@NelleV NelleV left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks mostly good, but could you merge the figures into one using several subplots?

Streamplot
==========

Demo of the `streamplot` function.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you remove this line? It is not informative.

@patniharshit

Copy link
Copy Markdown
Contributor Author

figure_1
I don't know why the one wth masking is behaving like this. Can you point out how to fix it?

@WeatherGod

WeatherGod commented Mar 20, 2017

Copy link
Copy Markdown
Member

It is because of imshow() applying an equal aspect ratio.

@patniharshit

patniharshit commented Mar 21, 2017

Copy link
Copy Markdown
Contributor Author

Looks like this

@phobson

phobson commented Mar 21, 2017

Copy link
Copy Markdown
Member

@patniharshit

I prefer the appearance of the plot with an equal aspect ratio. To get this in a desirable way, I'd used the gridspec module (it's always nice to show that off in other places)

fig = plt.figure(figsize=(7, 9))
gs = gspec.GridSpec(nrows=3, ncols=2, height_ratios=[1, 1, 2])
ax0 = fig.add_subplot(gs[0, 0])
ax1 = fig.add_subplot(gs[0, 1])
ax2 = fig.add_subplot(gs[1, 0])
ax3 = fig.add_subplot(gs[1, 1])
ax4 = fig.add_subplot(gs[2:, :])
ax4.set_aspect('equal')

gs

@patniharshit

Copy link
Copy Markdown
Contributor Author

figure_1-1

@dstansby dstansby added this to the 2.0.1 (next bug fix release) milestone Mar 24, 2017
fig.colorbar(strm.lines)
ax3.set_title('Controlling Starting Points')

# Displaying the starting points with red symbols.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

They're blue, not red.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

Y, X = np.mgrid[-w:w:100j, -w:w:100j]
U = -1 - X**2 + Y
V = 1 + X - Y**2
speed = np.sqrt(U*U + V*V)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If I'm not mistaken, these are the same values as the ones at the top. Maybe give the ones for ax3 a different name, then you won't need to recreate these same values again here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oops, this comment is supposed to be about lines 62-65.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, U,V are being changed at line 47-49.

@afvincent afvincent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In overall the PR seems nice to me, thanks for working on this @patniharshit :)! I still have some comments and suggestion. Some are taken from #8082. Again, if you are a native English speaker and disagree with some of my remarks about phrasing: drop them ;).

FWIW, switching from cmap=plt.cm.name_of_a_cmap to cmap='name_of_a_cmap' is just a suggestion, but I think it may make the script a (very little) bit simpler.

If I am correct, you will also need to change the references in the docs that points to the deleted files. Looking at #8082, it should now be:

  • in doc/users/screenshots.rst , l. 74: .. plot:: mpl_examples/images_contours_and_fields/streamplot_demo.py
  • in doc/users/prev_whats_new/whats_new_1.2.rst , l. 155: .. plot:: mpl_examples/images_contours_and_fields/streamplot_demo.py

Streamplot
==========
A streamplot, or streamline plot, is used to display 2D vector fields. This
example shows a few features of the stream plot function:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

stream plot <- streamplot?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.


* Varying the color along a streamline.
* Varying the density of streamlines.
* Varying the line width along a stream line.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

stream line <- streamline?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

* Varying the color along a streamline.
* Varying the density of streamlines.
* Varying the line width along a stream line.
* Controlling the start points of streamlines.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

start points <- starting points?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

ax4.set_title('Streamline with Masking')

ax4.imshow(~mask, extent=(-w, w, -w, w), alpha=0.5,
interpolation='nearest', cmap=plt.cm.gray, aspect='auto')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

plt.cm.gray <- 'gray'?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.


ax3 = fig.add_subplot(gs[1, 1])
strm = ax3.streamplot(X, Y, U, V, color=U, linewidth=2,
cmap=plt.cm.autumn, start_points=seed_points.T)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

plt.cm.autumn <- 'autumn'?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.


# Varying color along a streamline
ax1 = fig.add_subplot(gs[0, 1])
strm = ax1.streamplot(X, Y, U, V, color=U, linewidth=2, cmap=plt.cm.autumn)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

plt.cm.autmn <- 'autumn'?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

# Controlling the starting points of the streamlines
X, Y = (np.linspace(-3, 3, 100),
np.linspace(-3, 3, 100))
U, V = np.mgrid[-3:3:100j, 0:0:100j]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Any reason for changing the U, V compared to the former streamplot_demo_start_points.py script? How bad would it be to simply re-use the same X, Y, U and V as for the other subplots? This way, you'd just need to create these (dummy) data at the beginning of the script (NB: it is already the case at ll. 18-22) and after that, the script is focused on the plotting aspect of streamplot. I think that it is also close to what @QuLogic had in mind in one of his remarks.

@patniharshit patniharshit Mar 25, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have kept U, V exactly same as they are in previous streamplot_demo_start_points.py.

But if I comment out above lines and use same U,V everywhere then I get this plot -
figure_1-2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh I think that I understand why the current situation was weird for both of us!

The current example in the official docs is indeed the one that you are refering to. However, the starting point example was changed in #6672 (by myself, which is why I was having the feeling that you changed U and V here) to use the same data as the other streamplot examples, which I think is really more demonstrative to show the different plotting options of streamplot. The related version of the example is only in our devdocs gallery (I guess because the PR was milestoned for 2.1 and not backported into 2.0). So I would definitively support to use the same data (built once for all at the top of the script) for all the subplots :).

Y, X = np.mgrid[-w:w:100j, -w:w:100j]
U = -1 - X**2 + Y
V = 1 + X - Y**2
speed = np.sqrt(U*U + V*V)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If one decides to always plot the same data among all the different subplots, the 5 lines above could be deleted.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Huh, why is this comment here?! This is the opposite of what I had in mind ^^... Actually lines 19-23 would be the only place where I would suggest to create all the dummy data!

==========
Streamplot
==========
A streamplot, or streamline plot, is used to display 2D vector fields. This

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am not sure that it is a mandatory requirement, but you may want to add a blank line betwenn the second ===== line and the first line of the paragraph. Maybe @NelleV would be able to confirm what SG and rST really require :).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think it is.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Then one remaining argument in favor of adding a blank line would be consistency among our example scripts, as a lot of other examples in the docs seem to have one such blank line ;). But I am also fine with no blank line if SG and rST are OK!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Blank line added to maintain consistency among all examples.

# Displaying the starting points with blue symbols.
ax3.plot(seed_points[0], seed_points[1], 'bo')

ax3.axis((-3, 3, -3, 3))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I forgot to comment on this one during my review, sorry. Is this line really needed (I do not see the equivalent for the previous subplots)? If indeed it is and one actually uses the same dummy data accross all the subplots, I would then rather suggest using ax3.axis((-w, w, -w, w)) instead (if I understand correctly the purpose of this line).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Other subplots were going from -3 to 3 but this one was not, so probably this line was put there to have uniformity in all plots. I have kept this. But it can be deleted considering this is supposed to be an example, thus it should use minimal code to achieve target.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You are right. Due to the reduced amount of streamlines that are plotted, it seems like the axis is not exactly the same as for the other plots. So it might be wiser to keep it :). 👍 for the new version ax3.axis((-w, w, -w, w)).

@afvincent afvincent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A few remaining comments, among which my biggest one will be to use the same dummy data (defined at the beginning of the script) for all the subplots. But apart from that, the PR starts to look really good to me :).

Y, X = np.mgrid[-w:w:100j, -w:w:100j]
U = -1 - X**2 + Y
V = 1 + X - Y**2
speed = np.sqrt(U*U + V*V)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Huh, why is this comment here?! This is the opposite of what I had in mind ^^... Actually lines 19-23 would be the only place where I would suggest to create all the dummy data!

ax1 = fig.add_subplot(gs[0, 1])
strm = ax1.streamplot(X, Y, U, V, color=U, linewidth=2, cmap='autumn')
fig.colorbar(strm.lines)
ax1.set_title('Varying color')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Varying Color?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.


ax4 = fig.add_subplot(gs[2:, :])
ax4.streamplot(X, Y, U, V, color='r')
ax4.set_title('Streamline with Masking')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Plural form (as there are several streamlines), or "Streamplot with Masking"?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed to "Streamplot with masking".

==========

A stream plot, or stream line plot, is used to display 2D vector fields. This
example shows a few features of the stream plot function:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Again, when it comes to English, I may be totally wrong, but I think "the streamplot function" may be more correct than the current "the stream plot function", as it is refering to the streamplot function and not to a stream plot. I guess I was not clear enough: it was actually the meaning of my previous comment ^^. However, for the other utterances of "stream plot"/"stream line," I will let a native English speaker (who may be you!) decide if it is better than "streamplot"/"streamline" or not.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think generally, "streamline" is a word and "streamplot" is not, unless you're referring to the function itself (as in this case.)

@QuLogic

QuLogic commented Mar 25, 2017

Copy link
Copy Markdown
Member

@NelleV Should this be renamed as plot_something.py?

@QuLogic QuLogic modified the milestones: 2.1 (next point release), 2.0.1 (next bug fix release) Mar 25, 2017
@QuLogic

QuLogic commented Mar 25, 2017

Copy link
Copy Markdown
Member

I don't think example deletion should be backported, so I've upped the milestone.

@anntzer anntzer mentioned this pull request Mar 25, 2017
16 tasks
@afvincent

Copy link
Copy Markdown
Contributor

Travis failures seem unrelated (they are about test_mixedsubplots…)

@afvincent afvincent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Last tiny nitpicking, based on @QuLogic comment about writing “streamline” with a single word (basically it reverts some of the changes made by a530e79).

Besides, pinging @NelleV about the other @QuLogic's comment: should this be renamed as plot_something.py (for Sphinx gallery sake IIUC)?

Apart from that, the new version of the example script looks fine, thanks @patniharshit :)!

Question to core devs who may pass by: if @patniharshit is comfortable with git, would it be relevant to rebase, and squash a bit the commits?

Streamplot
==========

A stream plot, or stream line plot, is used to display 2D vector fields. This

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

streamline

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

All done.

A stream plot, or stream line plot, is used to display 2D vector fields. This
example shows a few features of the streamplot function:

* Varying the color along a stream line.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

streamline

example shows a few features of the streamplot function:

* Varying the color along a stream line.
* Varying the density of stream lines.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

streamlines


* Varying the color along a stream line.
* Varying the density of stream lines.
* Varying the line width along a stream line.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

streamline

* Varying the color along a stream line.
* Varying the density of stream lines.
* Varying the line width along a stream line.
* Controlling the starting points of stream lines.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

streamlines

* Varying the density of stream lines.
* Varying the line width along a stream line.
* Controlling the starting points of stream lines.
* Stream lines skipping masked regions and NaN values.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Streamlines

@afvincent afvincent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM :).

A few questions to other devs:

  • should this be renamed as “plot_something.py” for Sphinx gallery sake? (@NelleV , pinging our SG expert if she is around)
  • should it be rebased and some of the commits squashed if @patniharshit is comfortable with git?

@afvincent afvincent changed the title Merged streamline examples [MRG+1] Merged streamline examples Apr 5, 2017
@phobson

phobson commented Apr 5, 2017

Copy link
Copy Markdown
Member

@afvincent my answer is "yes" to both of those.

@patniharshit

Copy link
Copy Markdown
Contributor Author

Renamed, rebased and commits squashed.

@afvincent afvincent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor fixes needed. Apart from that, the other Travis failures seems to be spurious ones.

local intensity of the vector field.

.. plot:: mpl_examples/images_contours_and_fields/streamplot_demo_features.py
.. plot:: mpl_examples/images_contours_and_fields/streamplot_demo.py

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

'streamplot_demo.py' should be changed into 'plot_streamplot.py', shouldn't it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

Comment thread doc/users/screenshots.rst Outdated
such as the speed or local intensity of the vector field.

.. plot:: mpl_examples/images_contours_and_fields/streamplot_demo_features.py
.. plot:: mpl_examples/images_contours_and_fields/streamplot_demo.py

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Here too, 'streamplot_demo.py' should be changed into 'plot_streamplot.py', shouldn't it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

@patniharshit

Copy link
Copy Markdown
Contributor Author

Oh forgot this. Will do it asap.

@afvincent

Copy link
Copy Markdown
Contributor

LGTM. Thanks @patniharshit :) !

@QuLogic

QuLogic commented Apr 11, 2017

Copy link
Copy Markdown
Member

Ugh, I was going to merge this, but it looks like it needs a rebase with that sphinx-gallery PR merged.

@dstansby

Copy link
Copy Markdown
Member

I'm going to attempt a rebase and merge

@dstansby

Copy link
Copy Markdown
Member

Okay, that looks like it worked, but since it's the first time I've done a rebase and force-push I'll wait for CI to pass before merging.

@dstansby dstansby self-assigned this Apr 12, 2017
@dstansby dstansby merged commit b81576c into matplotlib:master Apr 12, 2017
@dstansby dstansby changed the title [MRG+1] Merged streamline examples Merged streamline examples Apr 12, 2017
@patniharshit patniharshit deleted the mergedStreamline branch April 12, 2017 11:59
@SKPsanjeevi

SKPsanjeevi commented Apr 28, 2017

Copy link
Copy Markdown

I see that the current version of the plot_streamplot.py from repo is giving errors/not executing.

@QuLogic

QuLogic commented Apr 28, 2017

Copy link
Copy Markdown
Member

I see no errors in the latest CI build; note that examples from master are not guaranteed to work on any released version.

@SKPsanjeevi

Copy link
Copy Markdown

How can I also replicate it? I am spending time on fixing the error from master. Could you please let me know, how to perform such a check?

@QuLogic

QuLogic commented Apr 28, 2017

Copy link
Copy Markdown
Member

Replicate what? There is no error in master.

@SKPsanjeevi

Copy link
Copy Markdown

I get the following error:

Traceback (most recent call last):
  File "plot_streamplot.py", line 50, in <module>
    cmap='autumn', start_points=seed_points.T)
  File "/usr/lib/python2.7/dist-packages/matplotlib/__init__.py", line 1814, in inner
    return func(ax, *args, **kwargs)
  File "/usr/lib/python2.7/dist-packages/matplotlib/axes/_axes.py", line 4464, in streamplot
    zorder=zorder)
  File "/usr/lib/python2.7/dist-packages/matplotlib/streamplot.py", line 140, in streamplot
    sp2[:, 0] += np.abs(x[0])
ValueError: operands could not be broadcast together with shapes (6,) (100,) (6,) 

@SKPsanjeevi

Copy link
Copy Markdown

The syntax for streamline says that x and y should be 1D array and u and v as 2D array. However in the example, both x and y are made as 2D grids using np.mgrid. When they are made just as 1D array, the issue is fixed.

@phobson

phobson commented Apr 28, 2017

Copy link
Copy Markdown
Member

@pssk1988 -- are you working with a matplotlib installed from the latest github master?

@SKPsanjeevi

SKPsanjeevi commented Apr 28, 2017

Copy link
Copy Markdown

@phobson : I don't think so :/ I will look for using the github matplotlib.

@SKPsanjeevi

Copy link
Copy Markdown

I am really sorry to bother you all without using the github version of matplotlib. Now it works fine.

@dstansby dstansby removed their assignment May 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants