Skip to content
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.

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

PEP8 fixes to colorbar.py #1336

Merged
merged 1 commit into from Oct 9, 2012
Merged

PEP8 fixes to colorbar.py #1336

merged 1 commit into from Oct 9, 2012

Conversation

NelleV
Copy link
Member

@NelleV NelleV commented Oct 5, 2012

Pep8 fixes on the module colorbar.

Thanks,
N

*False*. To manually update the ticks, call *update_ticks* method explicitly.
set tick locations. Tick locations are updated immediately unless
update_ticks is *False*. To manually update the ticks, call
*update_ticks* method explicitly.
Copy link
Member

Choose a reason for hiding this comment

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

This docstring needs a little massaging to work better with sphinx. I may be wrong, but I think update_ticks (the kwarg) needs to be encased in asterisks. And when referring to the update_ticks (the method), one usually uses meth:update_ticks. I'm mot entirely on the syntax of that, though.

Edit: clarity

Copy link
Member

Choose a reason for hiding this comment

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

This is "early" Sphinx syntax that we've adopted (as we were an early adopter, of course), so it's a bit different from what Numpy does, for example. (Hopefully we'll have a chance to go through and improve the consistency of docstrings at some point). Asterisks for kwargs, backticks for cross references to methods, functions, classes, etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

Should I modify this to "numpy" syntax or leave it as it is? The only modification I've done here is set the 79 caracter limit.

Copy link
Member

Choose a reason for hiding this comment

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

Keep this pull request just for PEP8 compliancy. As @mdboom points out, we should not only implement the changes for this docstring, but for all the other docstring inconsistencies as well.

@dmcdougall
Copy link
Member

Aside from the minor docstring gripes -- which are not PEP8 compliancy issues, but sphinx compliancy issues -- this gets my +1.

@mdboom
Copy link
Member

mdboom commented Oct 8, 2012

Aside from the comments already here, +1.

efiring added a commit that referenced this pull request Oct 9, 2012
@efiring efiring merged commit 4a902c4 into matplotlib:master Oct 9, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants