Skip to content

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

Merged the fill_demo figures and changed the axes#8407
jiyunshin wants to merge 2 commits into
matplotlib:masterfrom
jiyunshin:fill_demo_plot

Conversation

@jiyunshin

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

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

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.

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.

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.

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.

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.

use x, ax2, etc

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

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 phobson modified the milestones: 2.1 (next point release), 2.0.1 (next bug fix release) Apr 1, 2017
@jiyunshin

Copy link
Copy Markdown
Author

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

@phobson

phobson commented Apr 3, 2017

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?

@jiyunshin

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

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

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.

@dstansby

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 dstansby self-assigned this Apr 12, 2017
@dstansby

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

6 participants