Skip to content

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

Merged
story645 merged 3 commits into
matplotlib:masterfrom
chrissshe:improve_textbox_example
Feb 21, 2020
Merged

Improve documentation for examples/widgets/textbox.py#16549
story645 merged 3 commits into
matplotlib:masterfrom
chrissshe:improve_textbox_example

Conversation

@chrissshe

@chrissshe chrissshe commented Feb 18, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

@timhoffm timhoffm left a comment

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.

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

Comment thread examples/widgets/textbox.py Resolved
Comment thread examples/widgets/textbox.py Resolved
Comment thread examples/widgets/textbox.py Resolved
@tacaswell tacaswell added this to the v3.3.0 milestone Feb 18, 2020
story645
story645 requested changes Feb 18, 2020
View reviewed changes
Comment thread examples/widgets/textbox.py Resolved
@chrissshe

chrissshe commented Feb 19, 2020

Copy link
Copy Markdown
Contributor Author

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

@timhoffm

timhoffm commented Feb 19, 2020

Copy link
Copy Markdown
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

story645 commented Feb 19, 2020

Copy link
Copy Markdown
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

chrissshe commented Feb 19, 2020

Copy link
Copy Markdown
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

story645 commented Feb 19, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

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

chrissshe commented Feb 20, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@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

chrissshe commented Feb 20, 2020

Copy link
Copy Markdown
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

story645 commented Feb 21, 2020

Copy link
Copy Markdown
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