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: Persistence tests failing when using sklearn nightly #260

Conversation

BenjaminBossan
Copy link
Collaborator

This PR is urgent because CI for our PRs will fail until there is a fix.

After scikit-learn/scikit-learn#22094, sklearn estimators will contain an additional key in their __dict__ after loading, namely "__sklearn_pickle_version__". This causes our tests to fail, since they compare objects before and after loading.

The quick solution is to pop the item in our tests if it exists and only compare the remaining items.

Should the sklearn change be amended, we should remove the fix from this PR. Progress is tracked here:

scikit-learn/scikit-learn#25273

This PR is urgent because CI for our PRs will fail until there is a fix.

After scikit-learn/scikit-learn#22094, sklearn
estimators will contain an additional key in their __dict__ after
loading, namely "__sklearn_pickle_version__". This causes our tests to
fail, since they compare objects before and after loading.

The quick solution is to pop the item in our tests if it exists and only
compare the remaining items.

Should the sklearn change be amended, we should remove the fix from this
PR. Progress is tracked here:

scikit-learn/scikit-learn#25273
@BenjaminBossan BenjaminBossan added the bug Something isn't working label Jan 2, 2023
@BenjaminBossan BenjaminBossan changed the title Fix failing test that use sklearn nightly FIX: Persistence tests failing when using sklearn nightly Jan 2, 2023
@BenjaminBossan
Copy link
Collaborator Author

@skops-dev/maintainers this is ready for review.

Here is an example test run that currently fails because of this issue.

@adrinjalali adrinjalali merged commit 1b3c674 into skops-dev:main Jan 2, 2023
adrinjalali added a commit to adrinjalali/skops that referenced this pull request Jan 13, 2023
adrinjalali added a commit to adrinjalali/skops that referenced this pull request Jan 13, 2023
BenjaminBossan pushed a commit that referenced this pull request Jan 16, 2023
Revert "FIX: Persistence tests failing when using sklearn nightly (#260)"

This reverts commit 1b3c674.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants