-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Merged and improved the streamplot demonstration #8082
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
Conversation
…demo.py file. Corrected the incorrect description that accompanied the start_points demo in the streamplot example.
|
||
# Varying color along a streamline | ||
fig0, ax = plt.subplots() | ||
strm = ax.streamplot(X, Y, U, V, color=U, linewidth=2, cmap=plt.cm.autumn) |
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.
I have a slight preferences on using the colormap's name directly (ie "autumn").
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.
Sure. I was following the previous streamplot example, which used plt.cm.autumn. Otherwise I agree since it is less clutter.
If we want to limit the number of example renaming and moving around, I suggest we do not backport this to 2.0.0: we may have to rename the examples for the gallery for sphinx-gallery. attn @tacaswell (Note: I do not believe we should be worried about breaking people's bookmark so I don't think this should be a reason not to backport, but I also don't think we should be backporting documentation changes either.) |
cmap=plt.cm.autumn, start_points=seed_points.T) | ||
fig4.colorbar(strm.lines) | ||
|
||
ax.plot(seed_points[0], seed_points[1],'bo') |
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.
PEP8 violation: there is a space missing between seed_points[1]
, and 'bo'
.
@tmcclintock I think I prefer the solution of not backporting: this leaves us time to see what works and what doesn't in terms of naming and url. Unfortunately, the list of examples to merge and modify in that ticket is a huge under estimation of the work that need to be done in order to move to sphinx-gallery. We have over 400 examples to go over. Any work you do is very much appreciated! I am very happy to review any work done on the gallery and documentation. |
@NelleV ok, I think I follow. Excuse my ignorance at being a github newb. I can't say I know enough about the project to know what switching to a sphinx-gallery means/entails, so I guess at this point I can't digest your feedback. I'd be happy to do the rest of the items in the list, but if you are saying that they would never make it into the gallery anyway then I can look for other issues to try and tackle. |
@tmcclintock No, the question is just backport versus no backport, ie, should it go into matplotlib 2.0.1 or matplotlib 2.1. |
It looks like the documentaiton build is failing because there's still a link to Otherwise it looks good! |
@dstansby Hmm. I don't know where/how to fix that. Do you want me to do that now or is this something that is done external to a PR? |
|
||
seed_points = np.array([[-2, 0, 1], [-2, 0, 1]]) | ||
|
||
fig4, ax = plt.subplots() |
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.
Why fig4
and not fig3
?
If @dstansby is right, this page in the doc could be (of) the reason(s). Search “streamplot_demo_features.py” in doc/users/screenshots.rst to find the related link. But I have no idea of how to display only |
A quick way to find places that link to certain examples is
It looks like there's two places where the reference needs to be updated or removed. |
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.
LGTM.
I have a few minor comments but I will be happy to approve this PR for merging once my “biggest” questions will be answered :).
Edit: Well, it seems that my comments did not make it :/. I will readd them ASAP…
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.
Ok, here are my (mostly) minor comments this time…
fig1, (ax1, ax2) = plt.subplots(ncols=2) | ||
ax1.streamplot(X, Y, U, V, density=[0.5, 1]) | ||
|
||
# Varying the line width along a stream line |
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.
I would merge this block with the previous one. I find it a bit perturbing that the previous block creates two subplots but only uses one. IMO, the two description comments may then be merged into something like:
# Varying the density of streamlines (ax1) and the line width along
# a streamline (ax2).
|
||
* Varying the color along a streamline. | ||
* Varying the density of streamlines. | ||
* Varying the line width along a stream line. |
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.
stream line <- streamline?
Demo of the `streamplot` function. | ||
|
||
A streamplot, or streamline plot, is used to display 2D vector fields. This | ||
example shows a few features of the stream plot function: |
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.
stream plot <- streamplot
?
interpolation='nearest', cmap=plt.cm.gray) | ||
|
||
# Controlling the start points of streamlines | ||
X, Y = (np.linspace(-3, 3, 100), |
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.
Why not reuse the previous X
and Y
arrays? Especially if the purpose is to create similar arrays.
|
||
U, V = np.mgrid[-3:3:100j, 0:0:100j] | ||
|
||
seed_points = np.array([[-2, 0, 1], [-2, 0, 1]]) |
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.
Any reason for changing the U
, V
and seed_points
arrays compared to the former streamplot_demo_start_points.py
script?
ax.imshow(-mask, extent=(-w, w, -w, w), alpha=0.5, | ||
interpolation='nearest', cmap=plt.cm.gray) | ||
|
||
# Controlling the start points of streamlines |
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.
# Controlling the start points of streamlines
<- # Controlling the start points of the streamlines
. Disclaimer: I am not a native English speaker, so please forget this comment if I am obviously wrong ;)…
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.
The former refers to any streamlines, while the latter is about these streamlines. More importantly, I think it should be "starting" instead of "start".
|
||
fig3, ax = plt.subplots() | ||
strm = ax.streamplot(X, Y, U, V, color=U, linewidth=2, | ||
cmap="autumn", start_points=seed_points.T) |
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.
Being there, I would replace the double quotes with single ones (i.e. "autumn"
<- 'autumn'
), to be consistent with the rest of the script.
cmap="autumn", start_points=seed_points.T) | ||
fig3.colorbar(strm.lines) | ||
|
||
ax.plot(seed_points[0], seed_points[1], 'bo') |
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.
It might be useful to add a small comment to precise that this is some kind of an helper plot. Maybe simply:
# Displaying the starting points with red symbols.
Edit: start <- starting (see @QuLogic's comment above)
* Varying the density of streamlines. | ||
* Varying the line width along a stream line. | ||
* Streamlines skipping masked regions and NaN values. | ||
* Controlling the start points of streamlines. |
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.
See the similar comment below (in the relevant code block).
Hi @tmcclintock |
Normally superseded by #8336, which merges the ideas from this PR and another similar one :). Edit: marked as duplicate. I would be in favor of keeping it open until #8336 is finished and the comments done on current @tmcclintock's PR have been fixed in the new 2-in-1 PR. And thanks again @tmcclintock for opening this PR :). |
Superseded by #8336. |
This PR is to address issue #7956 .
Merged the three examples/images_contours_and_fields/streamplot_demo_X.py files into a single streamplot_demo.py file.
Additionally, in the previous streamplot_demo_start_points.py example, the comments in the header were inconsistent with the purpose of the example. This has been corrected in the new streamplot_demo.py example.