MNT Fix pre-commit #31013
Merged
MNT Fix pre-commit #31013
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Merged
e94835a to
fcf111e
Compare
[WARNING] repo `https://github.com/pre-commit/pre-commit-hooks` uses deprecated stage names (commit, push) which will be removed in a future version. Hint: often `pre-commit autoupdate --repo https://github.com/pre-commit/pre-commit-hooks` will fix this. if it does not -- consider reporting an issue to that repo.
Based on the exclude lists of black and ruff.
$ pre-commit run end-of-file-fixer -a fix end of files.........................................................Failed - hook id: end-of-file-fixer - exit code: 1 - files were modified by this hook Fixing CODE_OF_CONDUCT.md Fixing examples/miscellaneous/README.txt Fixing doc/whats_new/upcoming_changes/sklearn.inspection/29797.enhancement.rst Fixing sklearn/datasets/images/README.txt Fixing examples/gaussian_process/README.txt Fixing examples/frozen/README.txt Fixing examples/developing_estimators/README.txt Fixing examples/decomposition/README.txt Fixing examples/manifold/README.txt Fixing examples/inspection/README.txt Fixing doc/testimonials/README.txt Fixing examples/cross_decomposition/README.txt Fixing doc/modules/biclustering.rst Fixing doc/whats_new/upcoming_changes/sklearn.inspection/26202.enhancement.rst Fixing sklearn/svm/src/liblinear/linear.h Fixing sklearn/utils/src/MurmurHash3.cpp $
$ pre-commit run trailing-whitespace -a trim trailing whitespace.................................................Failed - hook id: trailing-whitespace - exit code: 1 - files were modified by this hook Fixing doc/logos/README.md Fixing doc/whats_new/v1.6.rst Fixing doc/modules/svm.rst Fixing doc/developers/maintainer.rst.template Fixing asv_benchmarks/benchmarks/config.json Fixing sklearn/datasets/tests/data/svmlight_classification.txt Fixing doc/modules/biclustering.rst Fixing doc/modules/feature_selection.rst Fixing sklearn/datasets/tests/data/svmlight_multilabel.txt $
fcf111e to
22eff44
Compare
adrinjalali
approved these changes
Mar 22, 2025
|
We run linters for two reasons:
|
Good point. However, that's only useful for linters ( I don't know if it's possible to run other pipelines (Azure, CircleCI and GitHub Actions) only if pre-commit.ci succeeds.
pre-commit does that. |
|
I actually did study As for getting the messages themselves for the bot, I don't mind moving to pre-commit. |
lucyleeow
pushed a commit
to lucyleeow/scikit-learn
that referenced
this pull request
Apr 2, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reference Issues/PRs
See also #30976.
What does this implement/fix? Explain your changes.
Fix the pre-commit configuration.
Any other comments?
It's not a good idea to run the linter and other tools from existing CI jobs and pre-commit. The main reason is that tools such as
blackoperate on a different fileset when run on a whole directory (black .) or run by pre-commit on each individual file separately (black sklearn/externals/_arff.py). Aligning teh exclude lists of black/ruff and pre-commit is tedious and error-prone.Instead, I would recommend running black/ruff only once in pre-commit instead of running these tools on a handful of different CI jobs (Azure, GitHub, etc.) for no reason (as far as I can see).