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

general pep8 fixes #3507

Merged
merged 2 commits into from Sep 13, 2014
Merged

general pep8 fixes #3507

merged 2 commits into from Sep 13, 2014

Conversation

cimarronm
Copy link
Contributor

I've begin fixing up pep8 style errors and updating the test test_coding_standards to reflect the fixes

@tacaswell
Copy link
Member

Did you do this by hand or with an auto-pep8 tool? (If it is an automatic tool I want at least one more person to read over this and if my hand I am happy to merge this now).

@cimarronm
Copy link
Contributor Author

This is by painstaking hand ;) I wouldn't trust a tool to be smart enough to do the right thing in every situation (of course, that doesn't mean I wouldn't make some stupid mistake as well).

I'm going to keep going through the code and try to clean things up for pep8 and get test_coding_standards to where it is checking everything.

@tacaswell
Copy link
Member

Great!

Do you want to keep accumulating changes here or merge this and do the next batch as a new PR?

I would suggest the second as it reduces the chances of picking up merge conflicts.

@cimarronm
Copy link
Contributor Author

That makes sense. Go ahead and merge and I'll do the next bath as a new PR. Thanks


- *dayfirst*: default is False so that MM-DD-YY has precedence over
DD-MM-YY. See http://labix.org/python-dateutil#head-b95ce2094d189a89f80f5ae52a05b4ab7b41af47
DD-MM-YY. See http://labix.org/python-dateutil\
Copy link
Member

Choose a reason for hiding this comment

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

don't split links. if a line starts as

# http://...

the pep8 tool should auto-skip the length check

Copy link
Contributor

Choose a reason for hiding this comment

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

unfortunately it does not do this yet

Copy link
Member

Choose a reason for hiding this comment

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

@thisch I think you just need to update your pep8. It seems to pass on travis and on my local machines.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm using the latest pep8 code from git.

# a b c d e f  http://labix.org/python-dateutil#head-b95ce2094d189a89f80f5ae52a05b4ab7b41af47

pep8 outputs

/tmp/tmp.py:1:80: E501 line too long (93 > 79 characters)

@cimarronm
Copy link
Contributor Author

I didn't know pep8 would skip those. Thanks, I learned something new. Updated to fix those.

tacaswell added a commit that referenced this pull request Sep 13, 2014
@tacaswell tacaswell merged commit b97abc9 into matplotlib:master Sep 13, 2014
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