Skip to content

Created some prose documentation about best practices #11657

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

Closed
wants to merge 1 commit into from

Conversation

WeatherGod
Copy link
Member

PR Summary

Add a "best practices" guide, and added a couple entries to it based on questions asked by attendees at SciPy.

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

@ImportanceOfBeingErnest
Copy link
Member

In how far should this be part of The how-to section? I suppose it's not really clear for a user where to look for such content if there are two documents with FAQs?

It also seems that what is written here as part of "Creating your own plotting functions" is already present in the Coding Styles section of the Usage tutorial.

@WeatherGod
Copy link
Member Author

My thinking is that this "Best Practices" document should merely be a summary. Meanwhile, documents like the style guide should be much more detailed and in-depth.

Also, the section for the function design might not make sense to be in a "style" guide now that we would have this best practices guide.

@tacaswell tacaswell added this to the v2.2-doc milestone Jul 15, 2018
fig.clear()

Now, only one figure object is ever made, and it is cleaned of any plots
at each iteration, so the memory usage will not grow.
Copy link
Contributor

Choose a reason for hiding this comment

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

... or just call plt.close("all") at the end of the loop. This avoids having to factor out figure creation from the main loop.

Copy link
Member

@timhoffm timhoffm Jul 15, 2018

Choose a reason for hiding this comment

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

👍 for close. It's more clear and (currently) even faster.

%%timeit
fig = plt.figure()
for i in range(50):
    plt.plot([1, 3, 2])
    plt.savefig('test.png')
    fig.clear()

3.12 s ± 53.8 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

%%timeit
for i in range(50):
    plt.figure()
    plt.plot([1, 3, 2])
    plt.savefig('test.png')
    plt.close()

2.75 s ± 27.1 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

It's also a good general reminder, not only with looping: Close your pyplot figures when your done.

Copy link
Member Author

Choose a reason for hiding this comment

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

What prompted me to make this document was when someone came to me with a script that was growing in memory significantly and they did something like this. I don't remember if they were calling plt.close() or not, but recycling the figure definitely solves that problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

Now that #12450 is merged one can also just create Figure()s completely bypassing pyplot to avoid the issue; perhaps this should be coordinated with #12628/#13062.

Now you want to make this plot for each of your data files. One might just
put the above code into a for-loop over all of your filenames. However, you
will eventually encounter a warning ``More than X figures hae been opened.``.
With enough looping, Python can eventually run out of memory, and calling
Copy link
Member

@timhoffm timhoffm Jul 15, 2018

Choose a reason for hiding this comment

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

Proposed rewording:

With enough looping, Python can eventually run out of memory because the pyplot figure manger keeps a reference to every created figure.

fig.clear()

Now, only one figure object is ever made, and it is cleaned of any plots
at each iteration, so the memory usage will not grow.
Copy link
Member

@timhoffm timhoffm Jul 15, 2018

Choose a reason for hiding this comment

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

👍 for close. It's more clear and (currently) even faster.

%%timeit
fig = plt.figure()
for i in range(50):
    plt.plot([1, 3, 2])
    plt.savefig('test.png')
    fig.clear()

3.12 s ± 53.8 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

%%timeit
for i in range(50):
    plt.figure()
    plt.plot([1, 3, 2])
    plt.savefig('test.png')
    plt.close()

2.75 s ± 27.1 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

It's also a good general reminder, not only with looping: Close your pyplot figures when your done.

For nontrivial plots, it may make sense to create your own plotting
functions for easy reuse. Here are some guidelines:

* Have an ``ax`` positional argument as the first argument.
Copy link
Member

Choose a reason for hiding this comment

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

This pattern should be discussed.

IMO ax as the first positional argument is not a good choice. I would rather use something like:

def my_plotter(data1, data2, ax=None, param_dict=None):
    if ax is None:
        ax = plt.gca()
    ...

Usage patterns:

my_plotter(data1, data2)  # similar to pyplot
my_plotter(data1, data2, ax=ax)

See also #9111

Copy link
Member

Choose a reason for hiding this comment

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

This definitely should be discussed, because there is some discussion of a new interface that would have this pattern as the default for all plotting functions. ping @tacaswell @efiring

@jklymak
Copy link
Member

jklymak commented Apr 15, 2021

I'm going to close this. Much of this info has been incorporated elsewhere. It is also a grab-bag a-la the FAQ page and I really don't think we want grab-bag pages like this. If there is material here that is still relevant, please feel free to rebase and reopen or make a new PR. Thanks!

@jklymak jklymak closed this Apr 15, 2021
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.

7 participants