Skip to content

sklearn API change #1120

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
May 31, 2024
Merged

sklearn API change #1120

merged 7 commits into from
May 31, 2024

Conversation

@j-c-c
Copy link
Collaborator

@j-c-c j-c-c commented May 28, 2024

In a recent release sklearn altered the API of the PCA class methods _fit_full() and _fit_truncated(). Formerly, the namespace of the data was determined within these methods. Now, the namespace is determined in the method _fit() which calls the above methods and passes the namespace as an argument.

This API changed caused our subclass, ComplexPCA, of sklearn's PCA class to be broken.

In this PR I've made a similar change to our subclass ComplexPCA.

@j-c-c j-c-c added bug Something isn't working CI Continuous Integration extern Relating to external changes labels May 28, 2024
@j-c-c j-c-c self-assigned this May 28, 2024
@j-c-c j-c-c linked an issue May 28, 2024 that may be closed by this pull request
sklearn release breaking API #1119
Closed
@j-c-c
Copy link
Collaborator Author

j-c-c commented May 28, 2024

Looks like sklearn v1.5.0 only supports python 3.9-3.12. So I'll revert the changes and pin to usable version of sklearn until we're ready to move our python window.

@garrettwrong
Copy link
Collaborator

garrettwrong commented May 30, 2024

I'm going to change this to "Ready for Review" just to kick off the next round of CI jobs.

@garrettwrong garrettwrong marked this pull request as ready for review May 30, 2024 15:37
garrettwrong
garrettwrong approved these changes May 30, 2024
View reviewed changes
Copy link
Collaborator

@garrettwrong garrettwrong left a comment

Choose a reason for hiding this comment

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

Thanks for covering this CI patch!

@j-c-c j-c-c merged commit bd52df2 into develop May 31, 2024
@j-c-c j-c-c deleted the sklearn_bug branch May 31, 2024 13:44
@j-c-c j-c-c mentioned this pull request Jun 3, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@garrettwrong garrettwrong garrettwrong approved these changes

@janden janden Awaiting requested review from janden janden is a code owner

Assignees

@j-c-c j-c-c

Labels

bug Something isn't working CI Continuous Integration extern Relating to external changes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

sklearn release breaking API

3 participants

@j-c-c @garrettwrong