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

Update for sklearn 1.3. #77

Merged
merged 3 commits into from
Aug 12, 2024
Merged

Update for sklearn 1.3. #77

merged 3 commits into from
Aug 12, 2024

Conversation

wcwitt
Copy link
Collaborator

@wcwitt wcwitt commented Aug 12, 2024

No description provided.

@cortner
Copy link
Member

cortner commented Aug 12, 2024

Can you add J1.10 to the tests?

@wcwitt
Copy link
Collaborator Author

wcwitt commented Aug 12, 2024

Sure - for the moment I'm trying to get any test to pass.

@wcwitt
Copy link
Collaborator Author

wcwitt commented Aug 12, 2024

What is best practice, should I drop J1.9 when adding J1.10?

@wcwitt
Copy link
Collaborator Author

wcwitt commented Aug 12, 2024

@cortner this is ready for you. Both J1.9 and J1.10 are tested. We might lose backwards compatibility with earlier sklearn versions - I'm not entirely sure. But they definitely enforce max_iter as opposed to n_iter from sklearn 1.3. Once this is tagged I will need to make a few related adjustements in ACEpotentials.

@cortner
Copy link
Member

cortner commented Aug 12, 2024

We moved to 1.10 only for most packages but that was for convenience only. If you can get this to run on either then keep both.

@cortner
Copy link
Member

cortner commented Aug 12, 2024

release as breaking or as patch?

@cortner cortner merged commit 6e0f53e into main Aug 12, 2024
9 checks passed
@cortner cortner deleted the sklearn-update branch August 12, 2024 21:01
@wcwitt
Copy link
Collaborator Author

wcwitt commented Aug 12, 2024

Your call - I think it's technically breaking.

@cortner
Copy link
Member

cortner commented Aug 13, 2024

registered as 0.2.0

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.

2 participants