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

STYLE use types_or in pre-commit #38022

Closed
MarcoGorelli opened this issue Nov 23, 2020 · 7 comments · Fixed by #38457
Closed

STYLE use types_or in pre-commit #38022

MarcoGorelli opened this issue Nov 23, 2020 · 7 comments · Fixed by #38457
Assignees
Labels
Code Style Code style, linting, code_checks good first issue
Milestone

Comments

@MarcoGorelli
Copy link
Member

pre-commit 2.9 adds support for types_or, which would help clear up some of the less readable regexes like \.(py|pyx|rst)$, replacing them with types_or: [python, cython, rst]

@MarcoGorelli MarcoGorelli added Code Style Code style, linting, code_checks good first issue labels Nov 23, 2020
@rkc007
Copy link
Contributor

rkc007 commented Nov 23, 2020

Can I work on this issue?

@rkc007
Copy link
Contributor

rkc007 commented Nov 23, 2020

take

@MarcoGorelli
Copy link
Member Author

sure! the pre-commit docs may be useful here. You'll need to specify minimum_pre_commit_version somewhere and update the versions in requirements.txt and environment.yml. If any hook defines types upstream, it may be necessary to overwrite it with types: [text]. Anyway, if you open a PR I'll correct you if it's not right.

@MarcoGorelli
Copy link
Member Author

Hi @rkc007 - I've unassigned you so others can take this up if they wish, but if you're stil interseted in working on it do let me know and I'll re-assign you

@rkc007
Copy link
Contributor

rkc007 commented Dec 7, 2020

Hi @MarcoGorelli - Sorry I wasn't able to work on this issue as I was working on another issue #32073 and it is now approved. So, I would love to work on this issue now.

I have a quick doubt regarding this issue. I saw the documentation and your PR (pre-commit/pre-commit#1677) on the repository. Does the change here is like this -

types: [cython]

will be converted to

types_or: [cython]

but what about the files: \.pxi\.in$ ? Does this too change? If you could give an example from this figure, it would be great.
image

Thanks.

@MarcoGorelli
Copy link
Member Author

Hey - no worries, I've re-assigned you 😄

For flake8, we run it with different options on each type of file, so I'd leave it as it is.

For isort, we could overwrite types and then use types_or:

types: [text]  # overwrite types[python]
types_or: [python, cython]

For some of the local hooks, we can types_or instead of regexes like files: \.(py|pyx|pxd|pxi)$


To check what type a file is, you can use identify. E.g.:

$ identify-cli pandas/_libs/tslibs/conversion.pxd
["cython", "file", "non-executable", "text"]

$ identify-cli pandas/_libs/tslibs/parsing.pyx
["cython", "file", "non-executable", "text"]

$ identify-cli pandas/io/formats/info.py 
["file", "non-executable", "python", "text"]

@rkc007
Copy link
Contributor

rkc007 commented Dec 13, 2020

Thank you @MarcoGorelli for your guidance. I have created a PR for this issue. Can you please review it and let me know if I need to change anything in the PR.

@jreback jreback added this to the 1.3 milestone Dec 16, 2020
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 good first issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants