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

Addresses API changes in scikit-learn dependency #335

Merged
merged 11 commits into from
Jan 5, 2024

Conversation

bpkroth
Copy link
Collaborator

@bpkroth bpkroth commented Jan 4, 2024

@bpkroth
Copy link
Collaborator Author

bpkroth commented Jan 4, 2024

@amueller tried to fix this bug so we can use dabl for some MLOS integrations for a class demo.
I think this is the right approach, based on similar changes in scikit-learn: scikit-learn/scikit-learn@a794c58

However, the tests don't pass. But they don't pass in main right now either for me. Thoughts?

@amueller
Copy link
Collaborator

amueller commented Jan 4, 2024

Ideally dabl should work both with sklearn 1.1 and 1.3, which makes the fix a bit trickier. And the errors are because, well, main is supposed to run on 1.1 and not 1.3, I think? Thank you for tackling this, I haven't gotten around to it yet unfortunately.

setup.py Outdated Show resolved Hide resolved
@bpkroth
Copy link
Collaborator Author

bpkroth commented Jan 4, 2024

@amueller tried to fix this bug so we can use dabl for some MLOS integrations for a class demo. I think this is the right approach, based on similar changes in scikit-learn: scikit-learn/scikit-learn@a794c58

However, the tests don't pass. But they don't pass in main right now either for me. Thoughts?

FWIW, the following more restricted tests still pass:

pytest -x dabl/tests/test_models.py dabl/tests/test_available_if.py dabl/tests/test_utils.py dabl/tests/test_explain.py

@bpkroth bpkroth mentioned this pull request Jan 5, 2024
@bpkroth
Copy link
Collaborator Author

bpkroth commented Jan 5, 2024

FWIW, my personal fork has this set of changes passing for the full suite of test versions now here:
https://github.com/bpkroth/dabl/actions/runs/7426692519

Though, that relies on changes from #336 as well to do that test.

@amueller amueller merged commit 8e902c1 into dabl:main Jan 5, 2024
7 checks passed
@bpkroth bpkroth deleted the sklearn-deps-fixups branch January 5, 2024 21:33
bpkroth added a commit to microsoft/MLOS that referenced this pull request Jan 16, 2024
Adds a basic `mlos_viz.plot(exp)` style API for simple visualizations of
`ExperimentData` results relative to the experiment's objectives
(building off of #628 and dabl/dabl#335).

Note: this PR currently omits unit tests for the new module due to the
complexity of testing visualizations. We intend to add this in future
PRs. There is however, a working example of its use here right now:
Microsoft-CISL/sqlite-autotuning#41

---------

Co-authored-by: Sergiy Matusevych <sergiy.matusevych@gmail.com>
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.

URGENT: ImportError Issue with detect_types Module from dabl Library
2 participants