FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Doc changes to plt.show by fredrik-1 · Pull Request #11720 · matplotlib/matplotlib · GitHub

  1. matplotlib
  2. matplotlib

Repository navigation

Doc changes to plt.show - #11720

Closed
fredrik-1 wants to merge 1 commit into
matplotlib:masterfrom
fredrik-1:doc_plt.show
ConversationCommits (1)ChecksFiles changed
Closed

Doc changes to plt.show#11720
fredrik-1 wants to merge 1 commit into
matplotlib:masterfrom
fredrik-1:doc_plt.show

Conversation

fredrik-1 commented Jul 21, 2018

Copy link
Copy Markdown
Contributor

Rewrote the pyplot.show docstring and made the single kwarg block explicit.

Are the block keyword still experimental?

  • 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 commented Jul 21, 2018

Copy link
Copy Markdown
Member

There was already an attempt of making the block argument explicit in #11170 It didn't work out for some reason. Maybe it'll help to get it straight here though.

ImportanceOfBeingErnest reviewed Jul 21, 2018
View reviewed changes
Comment thread lib/matplotlib/pyplot.py
non-interactive to interactive mode (not recommended). In
that case it displays the figures but does not block.
Mainly used in non-interactive mode to draw and display
the figures. It usually does nothing in interactive mode.

ImportanceOfBeingErnest Jul 21, 2018

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Why would it not do anything in interactive mode? I think it also shows the figure there.

fredrik-1 Jul 21, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I am not sure. It was written in the old docstring that it does nothing.

When I test it in spyder, the only difference is that the "thumbnail" in the toolbar start to blink if plt.show() is used. The figure window don't pop up to the front or something like that.

fredrik-1 commented Jul 21, 2018

Copy link
Copy Markdown
Contributor Author

I didn't know about #11170. It doesn't seems clear what happens when reading that though. So the problem might be in one or several backends?

ImportanceOfBeingErnest commented Jul 21, 2018 •
edited
Loading

Copy link
Copy Markdown
Member

It looks like the exact same issue in the failing tests, if I recall that correctly, which is about the WebAgg backend. See https://github.com/matplotlib/matplotlib/pull/11170/files#r186467036.

fredrik-1 commented Jul 21, 2018

Copy link
Copy Markdown
Contributor Author

Yes, the WebAgg doesn't seem to work but isn't it solved by the the line that tacaswell comment? I try that now.

I don't know much about the different backends and how the backend class hierarchy works but to me it seems that the problem is that the show in backend bases take a keyword block but that this function is overridden in the WebAgg class with a show that takes no arguments.

WebAgg is taken care of in the backend_bases show.

timhoffm reviewed Jul 21, 2018
View reviewed changes
Comment thread lib/matplotlib/pyplot.py
----------

block : bool or None
True means that the execution is blocked until the figures are closed

timhoffm Jul 21, 2018

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Can be shortend: "Whether the execution is blocked until the figures are closed."

timhoffm Oct 13, 2018

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Alternative suggestion:

If True, the execution is blocked until all displayed figures are closed. If False, the python execution continues after the figures are displayed.

Comment thread lib/matplotlib/pyplot.py
and False means that the python execution continues after the figures
are displayed.

The default behavior is different in interactive and non-interactive

timhoffm Jul 21, 2018

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Shorter:

The default is True unless interactive mode is enabled or %matplotlib_ is used in IPython.

timhoffm Oct 13, 2018

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Wouldn't want to go for the long version here. It's just redundant and doesn't add to the clarity.

fredrik-1 commented Jul 25, 2018

Copy link
Copy Markdown
Contributor Author

I added a simple test of the input but I am not sure about the docstrings suggestions (or my own versions for that matter). I think that the explaination in the docstring that was suggested where to short.

timhoffm commented Jul 26, 2018

Copy link
Copy Markdown
Member

I think that my docstring proposals contain the same amount of information, they are just less verbose. But a third opinion would be good.

ImportanceOfBeingErnest commented Jul 26, 2018

Copy link
Copy Markdown
Member

A third opinion was being asked for: I can confirm that the two proposed changes by @timhoffm do indeed contain the same information.
But I can see how the initial proposals are easier to understand by inexperienced users. (On the other hand, more experienced users may be irritated by the verbosity compared to the ususal doc style.) In the end it may be a matter of taste and consistency.

timhoffm commented Jul 14, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

This has been superseeded by

@fredrik-1 sorry this stalled. Anyway, thanks for the contribution.

timhoffm closed this Jul 14, 2020
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

timhoffm timhoffm left review comments
ImportanceOfBeingErnest ImportanceOfBeingErnest left review comments

Assignees

No one assigned

Labels

Documentation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL