| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
…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.
I have a slight preferences on using the colormap's name directly (ie "autumn").
Sorry, something went wrong.
There was a problem hiding this comment.
Sure. I was following the previous streamplot example, which used plt.cm.autumn. Otherwise I agree since it is less clutter.
Sorry, something went wrong.
|
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.) |
Sorry, something went wrong.
| 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.
PEP8 violation: there is a space missing between seed_points[1], and 'bo'.
Sorry, something went wrong.
|
@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. |
Sorry, something went wrong.
|
@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. |
Sorry, something went wrong.
|
@tmcclintock No, the question is just backport versus no backport, ie, should it go into matplotlib 2.0.1 or matplotlib 2.1. |
Sorry, something went wrong.
|
It looks like the documentaiton build is failing because there's still a link to streamplot_demo_features somewhere in the documentation that needs to be removed or redirected to your new example. Otherwise it looks good! |
Sorry, something went wrong.
|
@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? |
Sorry, something went wrong.
|
|
||
| seed_points = np.array([[-2, 0, 1], [-2, 0, 1]]) | ||
|
|
||
| fig4, ax = plt.subplots() |
There was a problem hiding this comment.
Why fig4 and not fig3?
Sorry, something went wrong.
|
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 fig0 and fig1 (as previously) in the page with the new all-in-one version of the script. @NelleV would you know how to achieve this? |
Sorry, something went wrong.
|
A quick way to find places that link to certain examples is $ grep -r "streamplot_demo_features" doc/ lib/ doc//users/prev_whats_new/whats_new_1.2.rst:.. plot:: mpl_examples/images_contours_and_fields/streamplot_demo_features.py doc//users/screenshots.rst:.. plot:: mpl_examples/images_contours_and_fields/streamplot_demo_features.py It looks like there's two places where the reference needs to be updated or removed. |
Sorry, something went wrong.
There was a problem hiding this comment.
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…
Sorry, something went wrong.
There was a problem hiding this comment.
Ok, here are my (mostly) minor comments this time…
Sorry, something went wrong.
| 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.
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).
Sorry, something went wrong.
|
|
||
| * 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.
stream line <- streamline?
Sorry, something went wrong.
| 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.
stream plot <- streamplot?
Sorry, something went wrong.
| 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.
Why not reuse the previous X and Y arrays? Especially if the purpose is to create similar arrays.
Sorry, something went wrong.
|
|
||
| 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.
Any reason for changing the U, V and seed_points arrays compared to the former streamplot_demo_start_points.py script?
Sorry, something went wrong.
| 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.
# 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 ;)…
Sorry, something went wrong.
There was a problem hiding this comment.
The former refers to any streamlines, while the latter is about these streamlines. More importantly, I think it should be "starting" instead of "start".
Sorry, something went wrong.
|
|
||
| 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.
Being there, I would replace the double quotes with single ones (i.e. "autumn" <- 'autumn'), to be consistent with the rest of the script.
Sorry, something went wrong.
| 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.
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)
Sorry, something went wrong.
| * 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.
See the similar comment below (in the relevant code block).
Sorry, something went wrong.
|
Hi @tmcclintock |
Sorry, something went wrong.
|
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 :). |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
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.