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

Merged the fill_demo figures and changed the axes by jiyunshin · Pull Request #8407 · matplotlib/matplotlib · GitHub

Repository navigation

Merged the fill_demo figures and changed the axes - #8407

Closed
jiyunshin wants to merge 2 commits into
matplotlib:masterfrom
jiyunshin:fill_demo_plot
Closed

jiyunshin wants to merge 2 commits into
matplotlib:masterfrom
jiyunshin:fill_demo_plot

Conversation

Copy link
Copy Markdown

Improvement of gallery issue #7956
Merged fill_demo figure and fill_demo_features figure.
Modified the x-label and y-scaling.

daveh19 commented Mar 30, 2017

Copy link
Copy Markdown

My students did this pull request. The continuous integration failure appears to have nothing to do with their changes. They just wrote some examples for the gallery.

phobson 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.

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

This looks good, let's just clean up some of the variable names and remove redundant variables.

y2 = np.sin(3 * x1)

fig, ax = plt.subplots()
fig, (ax1, ax3) = plt.subplots(1,2, sharey=True)

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 we just call it ax2?

ax.fill(x, y, zorder=10)
ax.grid(True, zorder=5)
ax1.set_xticks([0, np.pi/2, np.pi, 3*np.pi/2, 2*np.pi])
ax1.set_xticklabels( ['$0$', r'$\frac{\pi}{2}$', r'$\pi$',r'$\frac{3\pi}{2}$',r'$2\pi$'] )

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 whitespace between ( and [ (same for closing sides)


x = np.linspace(0, 1, 500)
y = np.sin(4 * np.pi * x) * np.exp(-5 * x)
x1 = np.linspace(0, 2 * np.pi, 500)

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

let's make this x = np.linspace(0, 1, 500)


fig, ax = plt.subplots()
ax.fill(x, y1, 'b', x, y2, 'r', alpha=0.3)
x3 = np.linspace(0, 1, 500)

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

If we redefine x1 as i described above, we can remove this


ax3.set_title('fill_demo')
ax3.set_xlabel('Time', labelpad=8)
ax3.fill(x3, y3, zorder=10)

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

use x, ax2, etc

phobson added this to the 2.0.1 (next bug fix release) milestone Mar 31, 2017

QuLogic commented Apr 1, 2017

Copy link
Copy Markdown
Member

The title says "merged", but I don't see an old file being deleted?

(This would also mean 2.1, in my mind.)

phobson modified the milestones: 2.1 (next point release), 2.0.1 (next bug fix release) Apr 1, 2017

Copy link
Copy Markdown
Author

I left x1 since x = np.linspace(0, 1, 500) changes the original shape of the figure.

phobson commented Apr 3, 2017 •
edited
Loading

Copy link
Copy Markdown
Member

@jiyunshin Ah. you're right. I had to order of the args conflated.

Going back to @QuLogic 's comment, is there another example that should be removed since you're merging two examples?

Copy link
Copy Markdown
Author

We merged fill_demo.py and fill_demo_features.py into fill_demo.py.
I don't find fill_demo_features.py file in the code folder. Maybe someone already deleted?

phobson commented Apr 4, 2017

Copy link
Copy Markdown
Member

I'm a little confused. Where did you originally find the example to merge these two together?

NelleV commented Apr 4, 2017

Copy link
Copy Markdown
Member

It might be good to wait until #8247 is merged before doing further changes on the gallery. Some of these files have been renamed in this PR.

I wouldn't usually hold off PRs to wait for one to be merged, but this particular one has been ready to be merged for a while, and gets regular conflicts with master.

Copy link
Copy Markdown
Member

http://matplotlib.org/examples/lines_bars_and_markers/fill_demo_features.html is the example that seems to have disappeared in the master branch; I'm happy putting it back in here though.

This just needs a PEP8 clean and a rebase; let us know @jiyunshin if you would like someone else to do that, and thanks for the PR.

dstansby self-assigned this Apr 12, 2017

Copy link
Copy Markdown
Member

It looks like these have been merged by someone else: http://matplotlib.org/devdocs/gallery/lines_bars_and_markers/fill.html#sphx-glr-gallery-lines-bars-and-markers-fill-py

Thanks a lot for the PR though, and apologies I didn't get around to fixing it up before someone else made the change!

dstansby closed this Jul 25, 2017
dstansby removed their assignment Jul 31, 2017
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
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.

6 participants


Back | FazBrowse Home | New Git URL