-
-
Notifications
You must be signed in to change notification settings - Fork 210
[MNT] Update ruff and mypy version, and format files to match latest ruff checks #1553
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1553 +/- ##
==========================================
+ Coverage 52.71% 56.21% +3.49%
==========================================
Files 36 36
Lines 4325 4330 +5
==========================================
+ Hits 2280 2434 +154
+ Misses 2045 1896 -149 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| # sign up. Only accessing the data on OpenML does not require an account! | ||
| # | ||
| # If you don’t have an account yet, sign up now. | ||
| # If you dont have an account yet, sign up now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo, this should be "don't"
fkiraly
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thanks.
- please go through the files and check for problems or typos. I marked a few issue above.
- While we are at it, can you also ensure the target version for the checks is 3.10 or higher, so the
from __future__ import annotationsis not always added bypre-commit?
openml/evaluations/evaluation.py
Outdated
| "OpenML Run URL", | ||
| "Task ID", | ||
| "OpenML Task URL" "Flow ID", | ||
| "OpenML Task URLFlow ID", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo, missing comma?
|
fixed typos and bumped target version to 3.10 (previously 3.8) |
|
@fkiraly I also updated the mypy version to python 3.10 (previously 3.8), I know this is out of scope for this PR, but I would have had to made 44 changes if I continued with the older version. Hope that is okay? |
|
@satvshr, that is great! I was already wondering where the setting was that kept adding the |
fkiraly
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, but perhaps someone else should also look at whether there is anything they spot in the files.
Metadata
.gitignore#1547 (for ignoring.ruff_cache)ruffversion forpre-commit#1550Details
What does this PR implement/fix? Explain your changes.
ruff format .to align the codebase with the formatting rules of the updated Ruff versionruff check .checksnoqatags in places that will end up changing the architecture of the function/class if I try fixing it_to be compatible with RUF059This PR is going to be a bigger one in size but in my opinion, we should be compatible with the latest ruff version and get it over with sooner rather than later.
On a separate note, there are already a significant number of
noqatags in the codebase. We should consider revisiting the architecture of the functions and classes that rely on them to better align with Ruff’s best practices. Where alignment isn’t appropriate, we should at least discuss why those components don’t need to be Ruff-compatible.