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

[pre-commit.ci] pre-commit autoupdate #1223

Merged
merged 6 commits into from
Apr 25, 2023
Merged

Conversation

pre-commit-ci[bot]
Copy link
Contributor

@pre-commit-ci pre-commit-ci bot commented Feb 27, 2023

Copy link
Collaborator

@PGijsbers PGijsbers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to other contributors:

LGTM except for several mypy errors that need to be fixed: https://results.pre-commit.ci/run/github/17939040/1677540245.VvURZtjgSmyCCC9kJo-7Ww

Additionally, it looks like flake8 checks for a line length of 80, while our line limit should be 100 characters.

I can't address this right away, please assign yourself or leave a message if you are working on it.

@pre-commit-ci pre-commit-ci bot force-pushed the pre-commit-ci-update-config branch from 3e733eb to 15353ee Compare March 14, 2023 00:03
@pre-commit-ci pre-commit-ci bot force-pushed the pre-commit-ci-update-config branch from 46ce916 to d4151bc Compare April 4, 2023 01:14
@codecov-commenter
Copy link

codecov-commenter commented Apr 4, 2023

Codecov Report

Patch coverage: 88.88% and project coverage change: +0.36 🎉

Comparison is base (fb9f9eb) 85.24% compared to head (63dc0cc) 85.60%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1223      +/-   ##
===========================================
+ Coverage    85.24%   85.60%   +0.36%     
===========================================
  Files           38       38              
  Lines         5008     5009       +1     
===========================================
+ Hits          4269     4288      +19     
+ Misses         739      721      -18     
Impacted Files Coverage Δ
openml/_api_calls.py 88.70% <ø> (+2.25%) ⬆️
openml/datasets/dataset.py 87.28% <ø> (ø)
openml/extensions/extension_interface.py 91.66% <ø> (ø)
openml/extensions/sklearn/extension.py 90.34% <ø> (ø)
openml/flows/functions.py 84.74% <ø> (ø)
openml/study/study.py 72.00% <0.00%> (ø)
openml/tasks/functions.py 85.79% <ø> (ø)
openml/tasks/split.py 94.50% <ø> (ø)
openml/datasets/functions.py 90.16% <100.00%> (ø)
openml/exceptions.py 96.77% <100.00%> (+0.10%) ⬆️
... and 5 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@pre-commit-ci pre-commit-ci bot force-pushed the pre-commit-ci-update-config branch from 9afa80b to b6285e9 Compare April 10, 2023 23:48
@mfeurer
Copy link
Collaborator

mfeurer commented Apr 17, 2023

I just had a look at what went wrong here with flake8 and must say I'm puzzled. flake8 simply ignore the .flake8 file:

(openml) feurerm@mllap04:~/sync_dir/projects/openml/python$ flake8 --filename ./openml/evaluations/evaluation.py --verbose
flake8.checker            MainProcess     82 INFO     Making checkers
flake8.checker            MainProcess    139 INFO     Checking 1 files
flake8.main.application   MainProcess    153 INFO     Finished running
flake8.main.application   MainProcess    153 INFO     Reporting errors
flake8.main.application   MainProcess    154 INFO     Found a total of 4 violations and reported 4
./openml/evaluations/evaluation.py:85:80: E501 line too long (80 > 79 characters)
./openml/evaluations/evaluation.py:87:80: E501 line too long (80 > 79 characters)
./openml/evaluations/evaluation.py:91:80: E501 line too long (86 > 79 characters)
./openml/evaluations/evaluation.py:114:80: E501 line too long (89 > 79 characters)

and one has to instead pass it in manually:

(openml) feurerm@mllap04:~/sync_dir/projects/openml/python$ flake8 --filename ./openml/evaluations/evaluation.py --verbose --config=.flake8
flake8.checker            MainProcess     82 INFO     Making checkers
flake8.checker            MainProcess    133 INFO     Checking 1 files
flake8.main.application   MainProcess    147 INFO     Finished running
flake8.main.application   MainProcess    147 INFO     Reporting errors
flake8.main.application   MainProcess    148 INFO     Found a total of 0 violations and reported 0

@mfeurer
Copy link
Collaborator

mfeurer commented Apr 17, 2023

After looking into this a bit more, I found that this problem was introduced in PyCQA/flake8@65c8937 and that the previous commit PyCQA/flake8@00ca630 actually works. This can be tested by putting the commit hashes into the rev field of the file .pre-commit-config.yaml.

I will raise an issue with the flake8 project if someone can confirm this.

@LennartPurucker
Copy link
Contributor

LennartPurucker commented Apr 18, 2023

I will raise an issue with the flake8 project if someone can confirm this.

I was able to reproduce this issue! Using the older commit as a reference works for me.

With the changes I just pushed, all pre-commits pass:
image

@mfeurer mfeurer force-pushed the pre-commit-ci-update-config branch from a6d6cfe to 63dc0cc Compare April 18, 2023 13:43
@mfeurer
Copy link
Collaborator

mfeurer commented Apr 25, 2023

I forgot to post the reason why this failed and what I fixed: PyCQA/flake8#1630

@PGijsbers PGijsbers self-requested a review April 25, 2023 08:54
@mfeurer mfeurer merged commit f9412d3 into develop Apr 25, 2023
@mfeurer mfeurer deleted the pre-commit-ci-update-config branch April 25, 2023 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants