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

[IGNORE] setup precommit to play nice with bidsschema CI #1267

Closed
wants to merge 0 commits into from

Conversation

Remi-Gau
Copy link
Collaborator

@Remi-Gau Remi-Gau commented Aug 31, 2022

Final splinter from #1219

This PR aims to find a pre-commit config that is compatible with the linting configuration used for the bidsschema tools and does not break anything in CI.

FYI: I am working on my main branch to be able to show the output of pre-commit CI:
for example: https://results.pre-commit.ci/run/github/150988005/1661942215.jTaSkg0fQD2c-OyJfvjqGw

Note that all the changes to files were done through running pre-commit run -a

@Remi-Gau Remi-Gau changed the title [MNT] setup precommit to play nice with bidsschema CI [IGNORE] setup precommit to play nice with bidsschema CI Aug 31, 2022
@Remi-Gau Remi-Gau added the exclude-from-changelog This item will not feature in the automatically generated changelog label Aug 31, 2022
@Remi-Gau
Copy link
Collaborator Author

Black pre-commit CI is fine:

https://results.pre-commit.ci/run/github/150988005/1661942215.jTaSkg0fQD2c-OyJfvjqGw

black....................................................................Passed

But bidsschematools CI fails:

https://github.com/bids-standard/bids-specification/runs/8111715253?check_suite_focus=true#step:5:21

Run flake8 .
./bidsschematools/validator.py:98:17: BLK100 Black would make changes.
./bidsschematools/render.py:109:13: BLK100 Black would make changes.
./bidsschematools/render.py:473:100: E501 line too long (100 > 99 characters)
./bidsschematools/schema.py:276:38: BLK100 Black would make changes.
./bidsschematools/utils.py:110:20: BLK100 Black would make changes.
./bidsschematools/tests/test_validator.py:163:12: BLK100 Black would make changes.
./bidsschematools/tests/test_make_testdata.py:39:21: BLK100 Black would make changes.
./bidsschematools/tests/test_rules.py:98:38: BLK100 Black would make changes.
./bidsschematools/tests/test_render.py:62:28: BLK100 Black would make changes.

@Remi-Gau
Copy link
Collaborator Author

flake8 pre-commit CI fails:

flake8...................................................................Failed
- hook id: flake8
- exit code: 1

tools/mkdocs_macros_bids/macros.py:5:1: F401 'bidsschematools.utils' imported but unused

This is despite .flake8 telling it to ignore F401 errors

From memory removing the utils will lead to failures of the PDF build.

@Remi-Gau
Copy link
Collaborator Author

Failure in PDF build in circle CI

Traceback (most recent call last):
  File "/root/project/pdf_build_src/process_markdowns.py", line 22, in <module>
    from mkdocs_macros_bids import macros  # noqa: F401
  File "/root/project/pdf_build_src/../tools/mkdocs_macros_bids/__init__.py", line 1, in <module>
    from macros import (
ModuleNotFoundError: No module named 'macros'

Exited with code exit status 1

https://app.circleci.com/pipelines/github/bids-standard/bids-specification/5146/workflows/7d134fa6-96ba-41a2-866f-7a23c209a764/jobs/14716?invite=true#step-104-11

@sappelhoff
Copy link
Member

as discussed in the maintainers meeting, we could simply remove flake8-black.

One could also do a "format check" by running pre-commit in CI and complaining if there is a git diff:

pre-commit run --all-files || ( git status --short ; git diff ; exit 1 )

used here for example:

https://github.com/sappelhoff/pyprep/blob/c6b268b90e08af2d4740496414cad649344345fa/.github/workflows/python_tests.yml#L66-L70

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exclude-from-changelog This item will not feature in the automatically generated changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants