Skip to content

Cleanup xticks/yticks docstrings.#15789

Merged
timhoffm merged 1 commit into
matplotlib:masterfrom
anntzer:ticks
Dec 7, 2019
Merged

Cleanup xticks/yticks docstrings.#15789
timhoffm merged 1 commit into
matplotlib:masterfrom
anntzer:ticks

Conversation

@anntzer

@anntzer anntzer commented Nov 28, 2019

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

timhoffm commented Nov 28, 2019

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

anntzer commented Nov 28, 2019

Copy link
Copy Markdown
Contributor Author

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

@timhoffm

timhoffm commented Nov 28, 2019

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

anntzer commented Nov 28, 2019

Copy link
Copy Markdown
Contributor Author

"Set and/or get"?

@timhoffm

timhoffm commented Nov 28, 2019

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

anntzer commented Nov 28, 2019

Copy link
Copy Markdown
Contributor Author

sounds good to me, done

dstansby
dstansby reviewed Nov 29, 2019
View reviewed changes
Comment thread lib/matplotlib/pyplot.py Outdated
@anntzer

anntzer commented Nov 29, 2019

Copy link
Copy Markdown
Contributor Author

Also compressed the examples a bit.

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

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.

Comment thread lib/matplotlib/pyplot.py
Comment thread lib/matplotlib/pyplot.py
@anntzer

anntzer commented Dec 1, 2019

Copy link
Copy Markdown
Contributor Author

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

anntzer commented Dec 4, 2019

Copy link
Copy Markdown
Contributor Author

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