-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Write up the recommendation on plotting. #2304
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
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
b60855e
Write up the recommendation on plotting.
pingyeh 1f7ec76
Replace plt.show by fig.show.
pingyeh 6b38d26
Add PyCharm issue and move file to dev/ subdir.
pingyeh 94658b7
Add an import for example test to pass.
pingyeh f30406c
Merge branch 'master' into plot-doc
CirqBot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| # Recommended interface and behavior of a `plot` method | ||
|
|
||
| Here we recommend the input arguments, return value, and behavior of the | ||
| `plot` method of a class. | ||
|
|
||
| ## Requirements | ||
|
|
||
| 1. **Convenience to interactive users.** This is the highest priority. | ||
| Compared to being called in a batch script as a library (for composing | ||
| more complicated plots or other purposes), the `plot` method is mainly | ||
| used in interactive sessions like ipython, jupyther, colab, PyCharm, | ||
| and python interpreter. | ||
| 1. **Plot is customizable.** The plot should be customizable by the user after | ||
| `plot` returns. This is important because user may need to change the look | ||
| for presentation, paper, or just the style they prefer. One plot style does | ||
| not fit all. | ||
| 1. **No unnecessary messages in interactive sessions.** It should not produce | ||
| any warning/error messages in normal usages in an interactive sessions. | ||
| See [#1890](https://github.com/quantumlib/Cirq/issues/1890#issue-473510953) | ||
| for an example of such message. | ||
| 1. **No popups during tests.** It should not produce any pop-up windows during | ||
| tests. | ||
|
|
||
| ## Recommendation | ||
|
|
||
| The `plot` method must produce a plot when there is no arguments in an | ||
| interactive session. The recommended way to achieve that is illustrated in the | ||
| example below. | ||
|
|
||
| ```python | ||
| import matplotlib.pyplot as plt | ||
|
|
||
| class Foo: | ||
| ... | ||
| def plot(self, ax: plt.Axes=None, **plot_kwargs: Any) -> plt.Axes: | ||
| show_plot = not ax | ||
| if show_plot: | ||
| fig, ax = plt.subplots(1, 1) # or your favorite figure setup | ||
| # Call methods of the ax instance like ax.plot to plot on it. | ||
| ... | ||
| if show_plot: | ||
| fig.show() | ||
| return ax | ||
| ``` | ||
|
|
||
| This `plot` method works in 2 modes: *memory mode* and *interactive mode*, | ||
| signalled by the presence of the `ax` argument. When present, the method is | ||
| instructed to plot on the provided `ax` instance in memory. No plot is shown | ||
| on the screen. When absent, the code is in *interactive* mode, and it creates | ||
| a figure and shows it. | ||
|
|
||
| The returned `ax` instance can be used to further customize the plot if the | ||
| user wants to. Note that if we were to call `plt.show` instead of `fig.show`, | ||
| the customizations on the returned `ax` does not show up on subsequent call to | ||
| `plt.show`. | ||
|
|
||
| To satisfy requirement number 4, unit test codes should create an `ax` object | ||
| and pass it into the `plot` method like the following example. | ||
|
|
||
| ```python | ||
| def test_foo_plot(): | ||
| # make a Foo instance foo | ||
| figure, ax = plt.subplots(1, 1) | ||
| foo.plot(ax) | ||
| # assert on the content of ax here if necessary. | ||
| ``` | ||
|
|
||
| This does not produce a pop-up window because `fig.show` is not called. | ||
|
|
||
|
|
||
| ## Classes that produce multi-axes plot | ||
|
|
||
| Some classes contain complicated data and plotting on a single `ax` is | ||
| not sufficient. The `plot` method of such a class should take an optional | ||
| `axes` argument that is a list of `plt.Axes` instances. | ||
|
|
||
| ```python | ||
| class Foo: | ||
| ... | ||
| def plot(self, axes: List[plt.Axes]=None, **plot_kwargs: Any) -> List[plt.Axes]: | ||
| show_plot = not axes | ||
| if show_plot: | ||
| _, axes = plt.subplots(1, 2) # or your favorite figure setup | ||
| elif len(axes) != 2: # your required number of axes | ||
| raise ValueError('your error message') | ||
| # Call methods of the axes[i] objects to plot on it. | ||
| ... | ||
| if show_plot: | ||
| fig.show() | ||
| return axes | ||
| ``` | ||
|
|
||
| The reason we don't recommend passing a `plt.Figure` argument is that, the | ||
| `plot` method has no information on which `plt.Axes` objects to plot on if | ||
| there are more `plt.Axes` in the figure than what the method needs. The caller | ||
| is responsible for passing in correct number of `Axes` instances. | ||
|
|
||
| The `plot` method can be tested similarly. | ||
|
|
||
| ## PyCharm issue | ||
|
|
||
| As of this writing in October 2019, running a script calling a `plot` method | ||
| in PyCharm does not pop up a window with the figure. A call to `plt.show()` | ||
| is needed to show it. We believe this is a PyCharm-specific issue because | ||
| the same code works in Python interpreter. | ||
|
|
||
| ## References | ||
| * Issue #1890 "Plotting code should not call `show`" | ||
| * PR #2097 | ||
| * PR #2286 | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,6 +32,7 @@ User Documentation | |
| qudits | ||
| development | ||
| examples | ||
| dev/plotting | ||
|
|
||
|
|
||
| API Reference | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add an
import matplotlib.pyplot as pltline. The example testing test is failing becausepltis not imported at this point.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, though the tests are green before the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. That was a bug in the tests. Not it did fail before the move to a subdir of docs. Fixed in #2307