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

406 hades weekly fail - hotfix #410

Merged
merged 13 commits into from Aug 28, 2023
Merged

406 hades weekly fail - hotfix #410

merged 13 commits into from Aug 28, 2023

Conversation

egillax
Copy link
Collaborator

@egillax egillax commented Aug 25, 2023

Fixes #406

The issue was that in scikit-learn 1.3 they added a new feature to decision trees to deal with missing data. This broke our serialization of scikit-learn models to json. The reason this was not detected in our regular tests but in the weekly Hades tests is that there was inconsistency in how python environments were set up for testing. It was hardcoded in our regular test github actions to use python 3.7, which has gone out of support. This resulted in our tests using an older version of scikit-learn which didn't have this issue.

In this PR I've

  • Fixed the serialization of trees so they work both with scikit-learn 1.3 and older versions.
  • Simplified the actions so the setup of python env uses configurePython function for consistency
  • Update ubuntu runner to use 22.04 which is the latest long-term supported release. This was necessary since otherwise a system library needed to be updated for scipy to work correctly in python 3.11
  • Update README to mention currently supported python versions 3.8 and newer are supported

Now the tests test against the default python version in conda, which at this moment is python 3.11. Other option would be to hardcode somehere a different python version to test against. But I'd expect versions 3.8-3.11 to work currently.

@jreps could you review/approve this?

@egillax egillax requested a review from jreps August 25, 2023 16:16
@egillax egillax merged commit 86930ed into main Aug 28, 2023
8 of 10 checks passed
@egillax egillax deleted the 406-hades_weekly_fail branch November 30, 2023 14:42
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.

Released version currently failing unit tests
1 participant