Skip to content

Improve documentation for examples/widgets/textbox.py #16549

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Feb 21, 2020
Merged

Improve documentation for examples/widgets/textbox.py #16549

merged 3 commits into from
Feb 21, 2020

Conversation

chrissshe
Copy link
Contributor

@chrissshe chrissshe commented Feb 18, 2020 •
edited
Loading

PR Summary

The current documentation in textbox.py doesn't explicitly explain that this is an interactive GUI. Its name textbox may be easily confused with normal text with box, which is probably used more often. And it's hard to tell otherwise from the embedded screenshot. It fooled me for a good minute at least. I added a few notes in docstring and comment to stress the fact that this is different than just inserting a static text box. Also to bring more clarity to the purpose of the "submit" function.

This PR is also related to #11654

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 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

@chrissshe chrissshe mentioned this pull request Feb 18, 2020
Closed
timhoffm
timhoffm reviewed Feb 18, 2020
View reviewed changes
Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

Please do not leave trailing whitespace. Our style checker does not like that:

./examples/widgets/textbox.py:8:75: W291 trailing whitespace
./examples/widgets/textbox.py:9:79: W291 trailing whitespace
./examples/widgets/textbox.py:12:30: W291 trailing whitespace
./examples/widgets/textbox.py:13:39: W291 trailing whitespace
./examples/widgets/textbox.py:29:76: W291 trailing whitespace
./examples/widgets/textbox.py:31:23: W291 trailing whitespace

@tacaswell tacaswell added this to the v3.3.0 milestone Feb 18, 2020
story645
story645 requested changes Feb 18, 2020
View reviewed changes
@chrissshe
Copy link
Contributor Author

I don't see any specific error in the failed CI... help is appreciated!

@timhoffm
Copy link
Member

For travis, click on the failed job (flake8 in this case) and inspect the log:

./examples/widgets/textbox.py:9:52: W291 trailing whitespace

You can ignore failures in the nightly build.

Similarly, for CircleCI (which we use for doc-building), click on the failed job. Check the messages in the "Build documentation" section. Usually, it's a good idea to download that log and search for "WARNING" with a text editor. This gives:

/home/circleci/project/doc/gallery/widgets/textbox.rst:14: WARNING: py:obj reference target not found: submit
/home/circleci/project/doc/gallery/widgets/textbox.rst:19: WARNING: py:obj reference target not found: Textbox

@story645
Copy link
Member

Those are probably my fault 'cause I didn't properly format my snippet in rest. the guidelines are here: https://matplotlib.org/devel/documenting_mpl.html#writing-docstrings

Optimally on_submit should be cross referenced against it's docs. I don't know how to format the reference to submit - it should be differentiated from plaintext but it's a user written function.

@chrissshe
Copy link
Contributor Author

@timhoffm I did download the circleci log but I only searched for error not warning :( And I didn't proceed to check the Travis. Thank you for providing the details
@story645 I will look into the cross-referencing later today. If no other way around, maybe I'll just remove this specific referencing in the introduction.

@story645
Copy link
Member

story645 commented Feb 19, 2020 •
edited
Loading

Reading that warning message again, looks like `.on_submit` was found, which agrees with the cross link guideance since it was unambiguous.

To fix the warnings, I think the following replacements should work:

`Textbox` with with `.Textbox` 
 `submit` with :code: `submit`

@chrissshe
Copy link
Contributor Author

chrissshe commented Feb 20, 2020 •
edited
Loading

@story645 following that guideline you sent, I figured out the right way to do it. It has to be

matplotlib.widgets.TextBox and submit

@chrissshe
Copy link
Contributor Author

The new commit addressed all failures and I think it's ready to merge

@story645 story645 merged commit 2bdf82b into matplotlib:master Feb 21, 2020
@story645
Copy link
Member

Thanks for your contribution! 🚀

@chrissshe chrissshe deleted the improve_textbox_example branch February 21, 2020 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timhoffm timhoffm timhoffm left review comments

@story645 story645 story645 approved these changes

Assignees
No one assigned
Labels
Documentation
Projects
None yet
Milestone
v3.3.0
Development

Successfully merging this pull request may close these issues.
4 participants
@chrissshe @timhoffm @story645 @tacaswell