FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

merge streamplot_demo_features streamplot_demo_masking streamplot_demo_start_points by patniharshit · Pull Request #8155 · matplotlib/matplotlib · GitHub

  1. matplotlib
  2. matplotlib

Repository navigation

merge streamplot_demo_features streamplot_demo_masking streamplot_demo_start_points - #8155

Closed
patniharshit wants to merge 1 commit into
matplotlib:masterfrom
patniharshit:streamplotMerge
ConversationCommits (1)ChecksFiles changed
Closed

merge streamplot_demo_features streamplot_demo_masking streamplot_demo_start_points#8155
patniharshit wants to merge 1 commit into
matplotlib:masterfrom
patniharshit:streamplotMerge

Conversation

patniharshit commented Feb 26, 2017

Copy link
Copy Markdown
Contributor

refs #7956

QuLogic requested changes Feb 26, 2017
View reviewed changes

QuLogic left a comment •
edited
Loading

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Merging these three files does not just mean copy&paste together; redundant stuff needs to be removed and documentation needs to be made consistent.

Comment thread examples/images_contours_and_fields/streamplot_demo_features.py
@@ -31,3 +31,56 @@
ax2.streamplot(X, Y, U, V, density=0.6, color='k', linewidth=lw)

plt.show()

QuLogic Feb 26, 2017

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Remove; there's one at the end.

Comment thread examples/images_contours_and_fields/streamplot_demo_features.py

"""
================================
Streamplot function with masking

QuLogic Feb 26, 2017

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

You'll need to merge this docstring with the one at the beginning to make something coherent.

Comment thread examples/images_contours_and_fields/streamplot_demo_features.py
masked regions and NaN values.
"""
w = 3
Y, X = np.mgrid[-w:w:100j, -w:w:100j]

QuLogic Feb 26, 2017

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Can X and Y be reused from above?

Comment thread examples/images_contours_and_fields/streamplot_demo_features.py
ax.imshow(~mask, extent=(-w, w, -w, w), alpha=0.5,
interpolation='nearest', cmap=plt.cm.gray)

plt.show()

QuLogic Feb 26, 2017

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Remove.

Comment thread examples/images_contours_and_fields/streamplot_demo_features.py

"""
========================================
Streamplot function with starting points

QuLogic Feb 26, 2017

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Again, you need to merge this with the first docstring.

Comment thread examples/images_contours_and_fields/streamplot_demo_features.py
"""
Y, X = np.mgrid[-3:3:100j, -3:3:100j]
U = -1 - X**2 + Y
V = 1 + X - Y**2

QuLogic Feb 26, 2017

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Can any of these be reused from before?

afvincent commented Feb 27, 2017

Copy link
Copy Markdown
Contributor

Wouldn't this be a duplicate effort of #8082?

patniharshit commented Mar 10, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@afvincent @QuLogic Should I continue with the suggested changes in review?

tacaswell commented Mar 10, 2017

Copy link
Copy Markdown
Member

I suggest combining efforts with @tmcclintock . The comments between the two PRs are very similar. I suggest starting from the branch in #8082 , addressing both sets of comments and opening a new PR with the combined efforts.

afvincent commented Mar 10, 2017

Copy link
Copy Markdown
Contributor

@patniharshit I agree with @tacaswell on the idea of merging the two PRs. In the case where some review comments about English are overlapping between @QuLogic's ones and mine, just trust @QuLogic ;).

patniharshit mentioned this pull request Mar 19, 2017
Merged

QuLogic commented Mar 25, 2017

Copy link
Copy Markdown
Member

Replaced by #8336.

QuLogic closed this Mar 25, 2017
patniharshit deleted the streamplotMerge branch March 25, 2017 04:43
patniharshit restored the streamplotMerge branch May 22, 2017 07:16
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

QuLogic QuLogic requested changes

Assignees

No one assigned

Labels

status: duplicate

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL