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

Remove useless semicolons in "Introductory / Basic Usage" tutorial by gustavi · Pull Request #23796 · matplotlib/matplotlib · GitHub

Repository navigation

Remove useless semicolons in "Introductory / Basic Usage" tutorial - #23796

Merged
timhoffm merged 1 commit into
matplotlib:mainfrom
gustavi:gustavi-patch-1
Oct 21, 2022
Merged

timhoffm merged 1 commit into
matplotlib:mainfrom
gustavi:gustavi-patch-1

Conversation

gustavi commented Sep 1, 2022

Copy link
Copy Markdown
Contributor

PR Summary

Remove useless semicolons in tutorial https://matplotlib.org/stable/tutorials/introductory/usage.html#sphx-glr-tutorials-introductory-usage-py.

PR Checklist

Tests and Styling

  • [n/a] Has pytest style unit tests (and pytest passes).
  • [n/a] Is Flake 8 compliant (install flake8-docstrings and run flake8 --docstring-convention=all).

Documentation

  • [n/a] New features are documented, with examples if plot related.
  • [n/a] New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • [n/a] API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).

github-actions Bot left a comment

Copy link
Copy Markdown

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

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a while, please feel free to ping @matplotlib/developers or anyone who has commented on the PR. Most of our reviewers are volunteers and sometimes things fall through the cracks.

You can also join us on gitter for real-time discussion.

For details on testing, writing docs, and our review process, please see the developer guide

We strive to be a welcoming and open project. Please follow our Code of Conduct.

This comment was marked as outdated.

Copy link
Copy Markdown
Member

On second thought, that output is also in https://matplotlib.org/3.6.0/tutorials/introductory/quick_start.html#a-simple-example which is counter to what I expected so maybe they are useless...

There is clearly something I do not understand about sphinx-gallery here.

QuLogic commented Sep 1, 2022

Copy link
Copy Markdown
Member

Yes, @jklymak added these on purpose to suppress the output #21794, but they didn't seem to have worked.

timhoffm commented Sep 2, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

IMHO we should to discuss with sphinx-gallery to make showing outputs configurable. Even if the semicolon worked it can be a little confusing for users not familiar with it.

Ideally that option could be configured on a global/file/block basis and be overridden by the more specific ones; i.e. global=off, this block=on.

StefRe commented Sep 2, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

You can now configure capture_repr on file-by-file basis, see sphinx-gallery/sphinx-gallery#891. It just didn't have the time to apply this to the matplotlib docs.
So the plan is to globally turn off capturing and revert this in the couple of examples where it's needed, i.e. it basically just requires to go through the examples and tutorials and set the capturing where needed.

StefRe commented Sep 10, 2022

Copy link
Copy Markdown
Contributor

There is clearly something I do not understand about sphinx-gallery here.

@tacaswell : as I explained in sphinx-gallery/sphinx-gallery#891: putting a semicolon after the last expression doesn't work here as the code is first compiled into an AST and then the last expression is taken from the tree where the semicolon as code delimiter isn't present, see code.

StefRe mentioned this pull request Sep 11, 2022
2 tasks done

timhoffm commented Sep 24, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Superseded by #23978 .

@gustavi thank you for your effort, even though we found different solution to solve the issue.

timhoffm closed this Sep 24, 2022

StefRe commented Sep 24, 2022

Copy link
Copy Markdown
Contributor

@timhoffm in fact the PR is still valid - the semicolons are useless in this case (see #23796 (comment)) and should be removed (along with the corresponding flake ignore)

timhoffm reopened this Sep 24, 2022

Copy link
Copy Markdown
Member

@StefRe thanks for the correction.

tacaswell closed this Oct 20, 2022
tacaswell reopened this Oct 20, 2022

Copy link
Copy Markdown
Member

"power cycled" to get the azure to re-run

tacaswell added this to the v3.7.0 milestone Oct 20, 2022
timhoffm merged commit ad0a4c0 into matplotlib:main Oct 21, 2022

Copy link
Copy Markdown
Member

Thanks @gustavi, and congratulations on your first contribution to Matplotlib!

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
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.

6 participants


Back | FazBrowse Home | New Git URL