-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Various pep8 fixes - specifically targeting files which are failing travis pep8 tests #4155
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
Conversation
# ratio below this will be masked if they touch a | ||
# border. Suggested value 0.01 ; Use -1 to keep | ||
# all triangles. | ||
# Number of recursive subdivisions of the initial mesh for smooth plots. |
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.
In my opinion, whichever PEP8 check is requiring this change should be turned off. The code was perfectly readable as it was, and is less readable with the change.
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.
It's "E113 unexpected indentation". If we disable that then I think most everything is okay. Just a few E112 I see
@efiring Looks like it may be better to just retract this PR and see if you want to disable checking E113 for the pep8 tests on travis. |
# twice as fast as lut[xa]; | ||
# using the clip or wrap mode and providing an | ||
# output array speeds it up a little more. | ||
# twice as fast as lut[xa]; |
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.
This is referring to using "take" instead of indexing; I think the whole comment can be deleted now. No one is likely to re-implement this without using "take".
Mostly this looks good; it's really only some of the comment block reformatting that strikes me as a step backwards. I think the PR will be fine with only moderate changes (including removing the no longer needed comments that I flagged--might as well) even if we do disable E113. |
Note that the pep8 tests are currently failing on master because the pep8 tool got fixed on an old version (1.5.7) we should fix that end ensure that master pases the the pep8 tests correctly before merging any other pep8 work |
See #4150 Basically I think the tests are failing due to a too old version of pep8 being installed on travis. |
I unfixed the version of the pep8 tool on master. This should be rebased on current master before merging to ensure that the tests run with the latest version of pep8 which is what we are aiming for The latests test has been run with 1.5.7 at the time of writing 1.6.2 is the latest available version see https://travis-ci.org/matplotlib/matplotlib/jobs/52091597 |
82d2988
to
dfed541
Compare
Various pep8 fixes - specifically targeting files which are failing travis pep8 tests
No description provided.