Skip to content

CI consistently put | at start of line in pygrep hooks #39301

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

Closed
MarcoGorelli opened this issue Jan 20, 2021 · 1 comment · Fixed by #39304
Closed

CI consistently put | at start of line in pygrep hooks #39301

MarcoGorelli opened this issue Jan 20, 2021 · 1 comment · Fixed by #39304
Assignees
Labels
Code Style Code style, linting, code_checks good first issue
Milestone

Comments

@MarcoGorelli
Copy link
Member

MarcoGorelli commented Jan 20, 2021

Currently, the pygrep hooks have the "or" | symbol at the end of the line, which makes diffs longer than necessary and can cause bugs. It would be better to put that symbol at the beginning of the line.

For example, instead of

    -   id: non-standard-numpy.random-related-imports
        name: Check for non-standard numpy.random-related imports excluding pandas/_testing.py
        language: pygrep
        exclude: pandas/_testing.py
        entry: |
            (?x)
            # Check for imports from np.random.<method> instead of `from numpy import random` or `from numpy.random import <method>`
            from\ numpy\ import\ random|
            from\ numpy.random\ import

it should be

    -   id: non-standard-numpy.random-related-imports
        name: Check for non-standard numpy.random-related imports excluding pandas/_testing.py
        language: pygrep
        exclude: pandas/_testing.py
        entry: |
            (?x)
            # Check for imports from np.random.<method> instead of `from numpy import random` or `from numpy.random import <method>`
            from\ numpy\ import\ random
            |from\ numpy.random\ import

There's even an | symbol missing in the non-standard-imports-in-tests, which should be fixed (having the symbol at the beginning of the line would probably have made this easier to spot)

The file that needs changing is .pre-commit-config.yaml

@MarcoGorelli MarcoGorelli added Code Style Code style, linting, code_checks good first issue labels Jan 20, 2021
@gustavocmaciel
Copy link
Contributor

take

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