scikit-learn / scikit-learn

Sponsor

Sponsor scikit-learn/scikit-learn
Notifications
Star 46.2k
Fork 21.6k

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

Jump to bottom

MAINT Update pre-commit setup for black #20292

Merged

rth merged 3 commits into scikit-learn:main from rth:black-pre-commit

Jun 18, 2021

+5 −4

Member

rth commented Jun 17, 2021

Update pre-commit setup for black

Also run all flake8 checks enabled in config using pre-commit

Unless you had it in some other PR @thomasjpfan ?


          Update pre-commit setup for black

e837767

rth changed the title ~~Update pre-commit setup for black~~ MAINT Update pre-commit setup for black

thomasjpfan approved these changes

Member

thomasjpfan left a comment

LGTM

ogrisel reviewed

.pre-commit-config.yaml

  
            hooks:

            -   id: flake8

                types: [file, python]

                # only check for unused imports for now, as long as

                # the code is not fully PEP8 compatible

                args: [--select=F401]

ogrisel Jun 17, 2021
Member

This might be a problem when working on the examples which allow delayed import statements when using the notebook style code layout and the non-blackified files such as benchmark scripts, no?

ogrisel Jun 17, 2021
Member

https://github.com/scikit-learn/scikit-learn/blob/main/setup.cfg#L54-L57

rth Jun 17, 2021
Author Member

Hmm, because we don't want to use # noqa in examples? I think we should come up with a config that would make flake8 . just work; related #20296 .

But since this requires more discussion, let me revert the flake8 change, here having black setup would be already good.

rth Jun 17, 2021
Author Member

OK, then with per-file-ignores it should be fine.

ogrisel Jun 17, 2021 •

edited

Member

I approved the PR. Feel free to merge it assuming you tested it and it does not yield spurious rejections when working on the examples.

rth added 2 commits


          Revert flake8 changes

2a11521


          Revert "Revert flake8 changes"

b09b245

This reverts commit 2a11521.

ogrisel approved these changes

Member Author

rth commented Jun 17, 2021

Feel free to merge it assuming you tested it and it does not yield spurious rejections when working on the examples.

It runs flake8 on examples, and overall we currently have around 50 flake8 errors there #20296 . However it only runs on the modified files on commit. So it will only tell you to fix the included flake8 errors, if you edit one of those examples.

Member

thomasjpfan commented Jun 18, 2021

I opened #20298 to make sure that flake8 . works on main.

Member Author

rth commented Jun 18, 2021 •

edited

Thanks @thomasjpfan ! Merging this then as it would be quite useful to have the black pre-commit on PRs currently.

rth merged commit bf0da87 into scikit-learn:main

30 checks passed

30 checks passed

check

Details

triage

Details

Check build trigger

Details

Build wheel for cp${{ matrix.python }}-${{ matrix.platform_id }}-${{ matrix.manylinux_image }}

Details

triage_file_extensions

Details

Source distribution

Details

Upload to Anaconda

Details

LGTM analysis: C/C++ No code changes detected

Details

LGTM analysis: JavaScript No code changes detected

Details

LGTM analysis: Python No code changes detected

Details

ci/circleci: deploy Your tests passed on CircleCI!

Details

ci/circleci: doc Your tests passed on CircleCI!

Details

ci/circleci: doc artifact Link to 0/doc/_changed.html

Details

ci/circleci: doc-min-dependencies Your tests passed on CircleCI!

Details

ci/circleci: lint Your tests passed on CircleCI!

Details

codecov/patch Coverage not affected when comparing d44c7ae...b09b245

Details

codecov/project 97.98% (+0.00%) compared to d44c7ae

Details

scikit-learn.scikit-learn Build #20210617.56 succeeded

Details

scikit-learn.scikit-learn (Get Git Commit) Get Git Commit succeeded

Details

scikit-learn.scikit-learn (Linting) Linting succeeded

Details

scikit-learn.scikit-learn (Linux py37_conda_openblas) Linux py37_conda_openblas succeeded

Details

scikit-learn.scikit-learn (Linux pylatest_pip_openblas_pandas) Linux pylatest_pip_openblas_pandas succeeded

Details

scikit-learn.scikit-learn (Linux ubuntu_atlas) Linux ubuntu_atlas succeeded

Details

scikit-learn.scikit-learn (Linux32 debian_atlas_32bit) Linux32 debian_atlas_32bit succeeded

Details

scikit-learn.scikit-learn (Linux_Runs pylatest_conda_mkl) Linux_Runs pylatest_conda_mkl succeeded

Details

scikit-learn.scikit-learn (Ubuntu_Bionic py37_conda) Ubuntu_Bionic py37_conda succeeded

Details

scikit-learn.scikit-learn (Windows py37_conda_mkl) Windows py37_conda_mkl succeeded

Details

scikit-learn.scikit-learn (Windows py37_pip_openblas_32bit) Windows py37_pip_openblas_32bit succeeded

Details

scikit-learn.scikit-learn (macOS pylatest_conda_forge_mkl) macOS pylatest_conda_forge_mkl succeeded

Details

scikit-learn.scikit-learn (macOS pylatest_conda_mkl_no_openmp) macOS pylatest_conda_mkl_no_openmp succeeded

Details

rth deleted the rth:black-pre-commit branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment