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

MAINT: flake8 *.pyx files #14147

Closed
wants to merge 1 commit into from
Closed

Conversation

gfyoung
Copy link
Member

@gfyoung gfyoung commented Sep 4, 2016

closes #12995

flake8-ed *.pyx files and fixed errors.

Removed the E226 check because that inhibits pointers (e.g. char*). In addition, the check is not even universally accepted in Python.

@gfyoung gfyoung mentioned this pull request Sep 4, 2016
@codecov-io
Copy link

codecov-io commented Sep 4, 2016

Current coverage is 85.26% (diff: 100%)

No coverage report found for master at 3110a72.

Powered by Codecov. Last update 3110a72...5677916

@jreback
Copy link
Contributor

jreback commented Sep 4, 2016

not sure what u mean E226 is not universally accepted

pls turn it back on

@jreback jreback added the Code Style Code style, linting, code_checks label Sep 4, 2016
@jreback
Copy link
Contributor

jreback commented Sep 4, 2016

ok, forget about E226, its not so easy to remove.

can you add E128 back to the .pxi.in as well.

do
echo "linting -> pandas/$path"
flake8 pandas/$path --filename '*.pyx' --select=E501,E302,E203,E226,E111,E114,E221,E303,E128,E231,E126
flake8 pandas/$path --filename '*.pyx' --select=E501,E302,E203,E111,E114,E221,E303,E128,E231,E126
Copy link
Contributor

Choose a reason for hiding this comment

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

change this to not use a loop, IOW flake8 pandas --filename '*.pyx' --select=..... will catch all .pyx (even if we add/move them)

@jreback jreback added this to the 0.19.0 milestone Sep 4, 2016
@gfyoung
Copy link
Member Author

gfyoung commented Sep 5, 2016

@jreback : I'm actually surprised that all of these flake8 tests passed. 😄 In any case, do you think we can postpone the pxi.in changes until a separate PR? This pyx change is massive enough as it is.

@jreback
Copy link
Contributor

jreback commented Sep 5, 2016

no this needs fixing here as u edited the .pyx for algos and join which are auto generated and will revert when the pxi.in are changed

@gfyoung
Copy link
Member Author

gfyoung commented Sep 5, 2016

@jreback : Ah, fair enough. Will fix then.

flake8-ed *.pyx files and fixed errors.

Removed the E226 check because that inhibits
pointers (e.g. char*). In addition, the check
is not even universally accepted in Python.
@gfyoung
Copy link
Member Author

gfyoung commented Sep 6, 2016

@jreback : On second look, what do you mean that algos.pyx is auto-generated? I believe you mean algos*.pxi are auto-generated. Those are not the same files.

@jreback
Copy link
Contributor

jreback commented Sep 6, 2016

right the .pxi are included at cythonization time
so if u only edited .pyx then ok

@gfyoung
Copy link
Member Author

gfyoung commented Sep 6, 2016

@jreback : Nice! You got Appveyor to work and now block this PR 😄 😠

At least Appveyor is not failing because of my PR. Need to fix that 3.5 failure with the locale.

@jreback
Copy link
Contributor

jreback commented Sep 6, 2016

oh that's great!

and I stole from numpy a line in appveyor.yml
it cancels the previous if u push again :)
unlike Travis

@jreback jreback closed this in e54d4db Sep 6, 2016
@jreback
Copy link
Contributor

jreback commented Sep 6, 2016

thanks!

@gfyoung gfyoung deleted the pyx-flake8 branch September 6, 2016 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Style Code style, linting, code_checks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PEP: fix up *.pyx
3 participants