Skip to content
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

Changed fill.py example to be less misleading #11009

Merged
merged 3 commits into from Apr 12, 2018

Conversation

samvaughan
Copy link

PR Summary

I've changed /examples/lines_bars_and_markers/fill.py to be less misleading, following issues #5827 and #7956.

These plots were over-simplified because fill was acting just like fill_between, since the final line of the region to be filled was completely along the x-axis. I've changed the first example to be a simple polygon (as suggested in #5827) and the second to not have the final point at y=0 anymore. I've also outlined the region being filled, to make what's going on more obvious.

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@timhoffm
Copy link
Member

timhoffm commented Apr 9, 2018

Thanks for the update.

https://8797-1385122-gh.circle-artifacts.com/0/home/circleci/project/doc/build/html/gallery/lines_bars_and_markers/fill.html?highlight=fill

I don't find the polygon particularly instructive:

  • Start and endpoint are still at the same y.
  • I think we could leave out the grid and thus the zorder.

maybe simply

x = [0, 1, 3, 1]
y = [1, 2, 1, 0]

fig, ax = plt.subplots()
ax.fill(x, y)

Minor things:

  • Single blank lines in the code example are sufficient. No need for two blank lines.
  • For completeness, the first plot should get a plt.plot() as well.

@samvaughan
Copy link
Author

Sure, no problem. I've removed the grid and made the first example a square

x = [0, 1, 2, 1]
y = [1, 2, 1, 0]
 
fig, ax = plt.subplots()
ax.fill(x, y)

and outlined it in black with plt.plot(). I've removed the double spaces too.

@timhoffm
Copy link
Member

Sorry, I didn't mean plt.plot() but plt.show(). My bad. The outline does not add extra information in this first example. It's sufficient to illustrate this in the second example.

@samvaughan
Copy link
Author

No worries, just changed it!

Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jklymak jklymak changed the title Changed fill.py example to be less misleading, after issue #5827 and #7956 Changed fill.py example to be less misleading Apr 12, 2018
@jklymak jklymak merged commit 640ecd9 into matplotlib:master Apr 12, 2018
jklymak added a commit that referenced this pull request Apr 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants