Skip to content

Conversation

lesteve
Copy link
Member

@lesteve lesteve commented Jul 18, 2025

This doesn't seem used anymore and we use xp.isdtype instead.

Looks like the _array_api.isdtype was added before we decided to vendor array-api-compat: #26029.

Copy link

github-actions bot commented Jul 18, 2025

✔️ Linting Passed

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

Generated for commit: 9eb1be4. Link to the linter CI: here

@StefanieSenger
Copy link
Member

That is slightly off-topic, but I was just wondering whether isdtype was used in one of the tests in test_array_api.py and it wasn't. Shouldn't codecov then have detected that it's untested (which could have indicated to us that it's unused)?

Copy link
Member

@jeremiedbb jeremiedbb left a comment

Choose a reason for hiding this comment

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

Thanks !

@jeremiedbb
Copy link
Member

Shouldn't codecov then have detected that it's untested (which could have indicated to us that it's unused)?

codecov on PRs checks the coverage of the diff. So when its usage was replaced by xp.isdtype, it was not touched and codecov didn't complain. I think it also check the global coverage and complains if we've less than some threshold, but a single uncovered small function can't make us pass under the threshold.

@jeremiedbb jeremiedbb enabled auto-merge (squash) July 18, 2025 13:38
@lesteve
Copy link
Member Author

lesteve commented Jul 18, 2025

Looking at the coverage of _array_api.py I removed two other unused functions.

@lesteve lesteve changed the title MNT Remove unused _array_api.isdtype MNT Remove unused _array_api functions Jul 18, 2025
@lesteve lesteve changed the title MNT Remove unused _array_api functions MNT Remove unused utils._array_api functions Jul 18, 2025
@jeremiedbb jeremiedbb disabled auto-merge July 18, 2025 13:46
@jeremiedbb jeremiedbb enabled auto-merge (squash) July 18, 2025 13:46
@jeremiedbb jeremiedbb merged commit a048a40 into scikit-learn:main Jul 18, 2025
38 checks passed
@lesteve lesteve deleted the remove-our-isdtype branch July 18, 2025 14:34
lucyleeow pushed a commit to lucyleeow/scikit-learn that referenced this pull request Aug 22, 2025
@jeremiedbb jeremiedbb mentioned this pull request Sep 3, 2025
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants