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

Various pep8 fixes - specifically targeting files which are failing travis pep8 tests #4155

Merged
merged 5 commits into from Feb 28, 2015

Conversation

cimarronm
Copy link
Contributor

No description provided.

@cimarronm cimarronm changed the title Various pep8 fixes - specifically targeting travis pep8 failures Various pep8 fixes - specifically targeting files which are failing travis pep8 tests Feb 25, 2015
# 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.
Copy link
Member

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.

Copy link
Contributor Author

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

@cimarronm
Copy link
Contributor Author

@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];
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 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".

@efiring
Copy link
Member

efiring commented Feb 25, 2015

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.
I like seeing all those backslashes go away!

@jenshnielsen
Copy link
Member

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

@jenshnielsen
Copy link
Member

See #4150 Basically I think the tests are failing due to a too old version of pep8 being installed on travis.

@jenshnielsen jenshnielsen mentioned this pull request Feb 25, 2015
@jenshnielsen
Copy link
Member

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

efiring added a commit that referenced this pull request Feb 28, 2015
Various pep8 fixes - specifically targeting files which are failing travis pep8 tests
@efiring efiring merged commit 917efaa into matplotlib:master Feb 28, 2015
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