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

[examples] pep8 fix E26* #3668

Merged
merged 3 commits into from Oct 22, 2014
Merged

[examples] pep8 fix E26* #3668

merged 3 commits into from Oct 22, 2014

Conversation

twmr
Copy link
Contributor

@twmr twmr commented Oct 18, 2014

E261 at least two spaces before inline comment
E262 inline comment should start with ‘# ‘
E265 block comment should start with ‘# ‘
E266 too many leading ‘#’ for block comment

This PR also fixes some E211 (whitespace before ‘(‘ ) errors overseen by the autopep8/pep8 tool.

and minor modification of the output of autopep8 where appropriate
Signed-off-by: Thomas Hisch <t.hisch@gmail.com>
@twmr
Copy link
Contributor Author

twmr commented Oct 18, 2014

If it is too cumbersome to review I can split this PR into 2 parts

@@ -110,8 +110,6 @@ def __init__(self, sigma, fraction=0.5):
self.gauss_filter = GaussianFilter(sigma, alpha=1)
self.light_source = LightSource()
self.fraction = fraction
#hsv_min_val=0.5,hsv_max_val=0.9,
# hsv_min_sat=0.1,hsv_max_sat=0.1)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

unused code even if the comments are removed

@tacaswell tacaswell added this to the v1.5.x milestone Oct 18, 2014
@tacaswell
Copy link
Member

Are you doing any triaging on the examples while doing this?

@twmr
Copy link
Contributor Author

twmr commented Oct 19, 2014

Yes, first I want to fix all the white-space issues (E2*) and then the rest. I order the white-space fix PRs such that each PR can be easily reviewed. The next planned big PR fixes E231 (missing whitespace after ‘,’ ) and the one (or two) after that all other white-space issues.

@jenshnielsen
Copy link
Member

Btw someone should probably fix the warnings in the examples when you are done with the pep8 fixes. It would be nice if building the examples did not raise all these warnings. I will hold off doing anything about this until you are done with the pep8 fixes to avoid merge conflicts.

@twmr
Copy link
Contributor Author

twmr commented Oct 19, 2014

@jenshnielsen which warnings do you mean?

@jenshnielsen
Copy link
Member

Look at the travis output of building the docs. Several of the examples raises warnings when run

@twmr
Copy link
Contributor Author

twmr commented Oct 20, 2014

Strange. I can reproduce those warnings. I'll try to fix them once we are done with the pep8 fixes.

@twmr
Copy link
Contributor Author

twmr commented Oct 22, 2014

Any other comments?

@@ -22,7 +22,7 @@
for i in range(len(DATA)):
(x,y) = DATA[i]
(dd, dl, r, dr, dp) = dash_style[i]
#print 'dashlen call', dl
#print('dashlen call %d' % dl)
Copy link
Member

Choose a reason for hiding this comment

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

Yay for fixing syntax in comments!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Such stuff still has to be done manually :/

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