8000 MNT fix ruff type vs isinstance errors by adrinjalali · Pull Request #27039 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

MNT fix ruff type vs isinstance errors #27039

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.

Sign up for GitHub

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
Merged
merged 1 commit into from
Aug 9, 2023
Merged

MNT fix ruff type vs isinstance errors #27039

merged 1 commit into from
Aug 9, 2023

Conversation

adrinjalali
Copy link
Member

Fix issues raised by new ruff. Right now linting is failing on main

@github-actions
Copy link
github-actions bot commented Aug 9, 2023

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: 8d0d771. Link to the linter CI: here

betatim
betatim approved these changes Aug 9, 2023
View reviewed changes
Copy link
Member
@betatim betatim left a comment

Choose a reason for hiding this comment

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

LGTM.

I'd expect a comment in the code if the author (for some obscure reason) really wanted to check the type instead of the usual/normal isinstance.

lesteve
lesteve approved these changes Aug 9, 2023
View reviewed changes
Copy link
Member
@lesteve lesteve left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Fine with merging this.

Is there a reason not to pin ruff as with the other linting tools though? It could even be an additional lockfile like we do for the "standard" (build+test) CI builds.

@lesteve lesteve enabled auto-merge (squash) August 9, 2023 09:19
@lesteve lesteve merged commit 8e867b3 into scikit-learn:main Aug 9, 2023
@adrinjalali
Copy link
Member Author

@lesteve we can pin the version, but regarding the lock files, I've had difficulties dealing with them, they're large, and it's very confusing to manage them. So for now I rather stay out of it personally 😁

@adrinjalali adrinjalali deleted the ruff/fix branch August 9, 2023 20:42
@lesteve lesteve mentioned this pull request Aug 18, 2023
Merged
TamaraAtanasoska pushed a commit to TamaraAtanasoska/scikit-learn that referenced this pull request Aug 21, 2023
@adrinjalali @TamaraAtanasoska
MNT fix ruff type vs isinstance errors (scikit-learn#27039)
594802b
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Sep 18, 2023
@adrinjalali @glemaitre
MNT fix ruff type vs isinstance errors (scikit-learn#27039)
e6d8b49
jeremiedbb pushed a commit that referenced this pull request Sep 20, 2023
@adrinjalali @jeremiedbb
MNT fix ruff type vs isinstance errors (#27039)
75cdfec
REDVM pushed a commit to REDVM/scikit-learn that referenced this pull request Nov 16, 2023
@adrinjalali @REDVM
MNT fix ruff type vs isinstance errors (scikit-learn#27039)
17d1ef3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@betatim betatim betatim approved these changes

@lesteve lesteve lesteve approved these changes

Assignees
No one assigned
Labels
module:metrics module:model_selection No Changelog Needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@adrinjalali @betatim @lesteve
0