Skip to content

Cleanup xticks/yticks docstrings. #15789

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 1 commit into from
Dec 7, 2019
Merged

Cleanup xticks/yticks docstrings. #15789

merged 1 commit into from
Dec 7, 2019

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Nov 28, 2019

  • Don't bother listing call signatures (the normal function signature is
    fine).
  • Slightly tighten the parameters/return value descriptions.

PR Summary

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

@timhoffm
Copy link
Member

This will only be correct after #15788 is in. For the current state, there would be no hint thatthat labels without ticks don't work.

Also IMHO the MATLAB-like API should somehow be mentioned (that was part of the pupose of the call signature). Having the same function work as getter/setter depending on passing parameters is a bit unusual for Python. To be precise, while the return value is documented, it is not obvious that plt.xticks() the the canonical way of getting the ticks and labels.

@anntzer
Copy link
Contributor Author

anntzer commented Nov 28, 2019

The beginning of the docstring literally states "Get or set ...". I guess "Set and get" would be slightly more accurate?

@timhoffm
Copy link
Member

I guess "Set and get" would be slightly more accurate?

Technically, yes, but with the inexplicit "Set nothing and get", which is quite obscure.

@anntzer
Copy link
Contributor Author

anntzer commented Nov 28, 2019

"Set and/or get"?

@timhoffm
Copy link
Member

Formally correct, but ugly (which reflects the API). I'm +0.5 for just sticking with "Get or set" plus an extra sentence "Pass no argument to return the current value without modifying it."

@anntzer
Copy link
Contributor Author

anntzer commented Nov 28, 2019

sounds good to me, done

dstansby
dstansby reviewed Nov 29, 2019
View reviewed changes
@anntzer
Copy link
Contributor Author

anntzer commented Nov 29, 2019

Also compressed the examples a bit.

timhoffm
timhoffm reviewed Dec 1, 2019
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.

This will only be correct after #15788 is in. For the current state, there would be no hint thatthat labels without ticks don't work.

still holds, and it's not yet clear if that's going to be accepted.

@anntzer
Copy link
Contributor Author

anntzer commented Dec 1, 2019

happy to wait for the discussion on the other PR to be resolved first

@anntzer
Cleanup xticks/yticks docstrings.
3c4e8f6 
- Don't bother listing call signatures (the normal function signature is
  fine).
- Slightly tighten the parameters/return value descriptions.
@anntzer
Copy link
Contributor Author

anntzer commented Dec 4, 2019

Edited as needed following the rejection of #15788.

@timhoffm timhoffm added this to the v3.2.0 milestone Dec 7, 2019
@timhoffm timhoffm merged commit f53cc28 into matplotlib:master Dec 7, 2019
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Dec 7, 2019
@timhoffm
Backport PR matplotlib#15789: Cleanup xticks/yticks docstrings.
0f00395
@meeseeksmachine meeseeksmachine mentioned this pull request Dec 7, 2019
Merged
@anntzer anntzer deleted the ticks branch December 7, 2019 21:32
timhoffm added a commit that referenced this pull request Dec 8, 2019
@timhoffm
Merge pull request #15867 from meeseeksmachine/auto-backport-of-pr-15…
d0baa6b 
…789-on-v3.2.x

Backport PR #15789 on branch v3.2.x (Cleanup xticks/yticks docstrings.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dstansby dstansby dstansby left review comments

@timhoffm timhoffm timhoffm approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@anntzer @timhoffm @dstansby