Write up the recommendation on plotting. #2304
Conversation
Strilanc
left a comment
Strilanc
left a comment
There was a problem hiding this comment.
This looks fantastic, Ping. We should have more documents like this. Let's start a dev subdirectory of the docs dir.
| @@ -0,0 +1,97 @@ | |||
| # Recommended interface and behavior of a `plot` method | |||
There was a problem hiding this comment.
You should create a docs/dev directory and put this inside of it, since this documentation is intended for developers and not users. We can also move development.md into it (but be careful to update any references in sphinx configuration).
| 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`, |
There was a problem hiding this comment.
What happens if we return None when ax is not set? Pycharm seem to require plt.show() instead of fig.show().
There was a problem hiding this comment.
Once plt.show() is called, the figure and ax can no longer be customized (e.g., add grid, change an axis label, etc).
Per offline conversation, the problem is that it doesn't work in the PyCharm environment. The non-blocking fig.show causes the pop-up to not be shown. Is it actually shown but disappears immediately? Is there a setting that can change PyCharm's behavior?
Customizability of plots is not in the current requirements, but I think it is very important. I'd like to add it to the list of requirements.
There was a problem hiding this comment.
It's fine if we don't support pycharm and instead nag their devs to make it work.
| ```python | ||
| class Foo: | ||
| ... | ||
| def plot(self, ax: plt.Axes=None, **plot_kwargs: Any) -> plt.Axes: |
There was a problem hiding this comment.
Add an import matplotlib.pyplot as plt line. The example testing test is failing because plt is not imported at this point.
There was a problem hiding this comment.
Done, though the tests are green before the change.
There was a problem hiding this comment.
Good catch. That was a bug in the tests. Not it did fail before the move to a subdir of docs. Fixed in #2307
Strilanc
added
the
automerge
CirqBot
added
the
front_of_queue_automerge
|
Automerge cancelled: A status check is failing. |
CirqBot
removed
automerge
Strilanc
added
the
automerge
CirqBot
added
the
front_of_queue_automerge
|
Automerge cancelled: A status check is failing. |
CirqBot
removed
automerge
dabacon
added
the
automerge
CirqBot
added
the
front_of_queue_automerge
CirqBot
removed
automerge
5 participants
After talking to @maffoo, it seems we don't really use wikis for docs, but rather write markdown files in the docs/ directory. Here is the recommendation doc for plotting that captures the requirements and recommended practices for the
plotmethods.I have tested the recommended
plotmethod in interactive sessions in colab, ipython and python interpreter. They all work as expected. @Strilanc, can you test it in PyCharm?