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

Remove arg normalize from Lasso() (no longer exists) #183

Merged
merged 1 commit into from
Mar 3, 2024

Conversation

jdblischak
Copy link
Contributor

Closes #182. Thanks to @Y-Isaac for reporting the error and identifying the fix

The argument normalize was marked as deprecated in scikit-learn 1.0.0 and supposedly removed in 1.2 (I didn't verify exactly when it was removed). See #182 (comment) for more details

@omerwe would you be able to add a test case of polyfun.py --compute-h2-bins without --nnls-exact?

@omerwe omerwe merged commit ac63a4c into omerwe:master Mar 3, 2024
2 checks passed
@omerwe
Copy link
Owner

omerwe commented Mar 3, 2024

@jdblischak thanks as always!

It's been a few years, but I think the reason I didn't use Lasso in the test cases is because it was slightly non-deterministic, even when fixing the random state (small rounding errors). This might have been related to different versions of sklearn or something else, I'm not sure.

In any case, thanks for the pull request. The normalize flag in Lasso by definition didn't do anything in PolyFun, since Lasso receives a precomputed Gram matrix. I'm not sure why I included it in the first place, but it's definitely not needed.

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.

TypeError: __init__() got an unexpected keyword argument 'normalize'
2 participants