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

Fix failing mlr diagnostic test by adding new scikit-learn default tag #3273

Merged
merged 4 commits into from Jul 4, 2023

Conversation

remi-kazeroni
Copy link
Contributor

@remi-kazeroni remi-kazeroni commented Jul 3, 2023

Description

Second attempt to fix the failing test test_safe_tags_no_get_tags. An additional tag array_api_support was added to the list of DEFAULT_TAGS in the dependency (scikit-learn 1.3.0) and this needs to be reflected in our code.


Before you get started

Checklist

It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.


To help with the number of pull requests:

Copy link
Contributor

@valeriupredoi valeriupredoi left a comment

Choose a reason for hiding this comment

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

(draft PR approval, Klaus, keep your watch on 😆 ) Fine by me too - I still think we need to not close the test failure issue with whatever PR we merge, and let @schlunma decide what's the best approach to handle this (he might want to add additional test cases or code features). Also this was nowhere in the scikit-learn Changelog for 1.3.0 (unless I missed it), so am not even sure if the breaking behavior we see is supposed to be like this. TL;DR: am suspicious 😁

.github/workflows/test.yml Outdated Show resolved Hide resolved
Co-authored-by: Valeriu Predoi <valeriu.predoi@gmail.com>
@bouweandela bouweandela marked this pull request as ready for review July 4, 2023 07:04
@zklaus zklaus changed the title Fix failing mlr diagnostic test by adding missing tag Fix failing mlr diagnostic test by adding new scikit-learn default tag Jul 4, 2023
@zklaus zklaus merged commit e1a06dd into main Jul 4, 2023
6 checks passed
@zklaus zklaus deleted the fix_mlr_tags branch July 4, 2023 08:34
bouweandela added a commit that referenced this pull request Jul 4, 2023
…tag (#3273)

Co-authored-by: Bouwe Andela <b.andela@esciencecenter.nl>
Co-authored-by: Valeriu Predoi <valeriu.predoi@gmail.com>
jvegreg pushed a commit that referenced this pull request Jan 14, 2024
…tag (#3273)

Co-authored-by: Bouwe Andela <b.andela@esciencecenter.nl>
Co-authored-by: Valeriu Predoi <valeriu.predoi@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failing mlr diagnostic script test
4 participants