-
-
Notifications
You must be signed in to change notification settings - Fork 7.7k
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
Style fixes for secondary_axis. #14591
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think one could even spare the specific import from matplotlib.ticker
. Otherwise, LGTM. (Hopefully CI will agree…)
(And I would curious to know where the the numerous useless imports were coming from :).)
@@ -254,9 +235,7 @@ def draw(self, renderer=None, inframe=False): | |||
using the converter specified by | |||
`~.axes._secondary_axes.set_functions` (or *functions* | |||
parameter when axes initialized.) | |||
|
|||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By curiosity, is there a reason for removing the blank line after the docstring? It looks like the file already hosts a mix of “blank line”/“no blank line” after a docstring anyway…
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See PyCQA/pydocstyle#361 for how I tend to use blank lines (basically, use one if the body has other blank lines). Not really strict about that, but given that you asked, I made the changes throughout the module :)
There's a lot of unused imports :)
53b7239
to
ec6fe36
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The remaining specific import from mticker
has been removed. Fair enough for the blank lines/docstring story (I was just curious).
Approving, provided all CI tests on green.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry should have cleaned up those imports!
There's a lot of unused imports :)
PR Summary
PR Checklist