Skip to content

FIX Fix GridSearchCV regression in 1.5 with parameter grid with heterogeneous parameter values #29078

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

Merged
merged 5 commits into from
May 23, 2024

Conversation

lesteve
Copy link
Member

@lesteve lesteve commented May 22, 2024

Reference Issues/PRs

Fix #29074

What does this implement/fix? Explain your changes.

np.result_type can raise ValueError in some cases e.g. np.result_type(None, {'a': '1'}). In that case we should use dtype object.

I haven't added a test yet but will do soon. A test has been added.

Copy link

github-actions bot commented May 22, 2024

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: 43ac226. Link to the linter CI: here

@lesteve
Copy link
Member Author

lesteve commented May 22, 2024

I have added a test based on the original report.

I also added a changelog, but there is definitely some room for improvement.

@lesteve lesteve changed the title FIX Fix GridSearchCV regression in 1.5 with nested grid FIX Fix GridSearchCV regression in 1.5 with parameter grid with heterogenous parameter values May 22, 2024
@lesteve lesteve changed the title FIX Fix GridSearchCV regression in 1.5 with parameter grid with heterogenous parameter values FIX Fix GridSearchCV regression in 1.5 with parameter grid with heterogeneous parameter values May 22, 2024
@lesteve lesteve added this to the 1.5.1 milestone May 22, 2024
@lesteve lesteve added the To backport PR merged in master that need a backport to a release branch defined based on the milestone. label May 22, 2024
Copy link
Member

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@jeremiedbb jeremiedbb left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks

@jeremiedbb jeremiedbb merged commit 1bd9c1d into scikit-learn:main May 23, 2024
33 checks passed
@Jacob-Stevens-Haas
Copy link

Thanks! Ran into this today

Jacob-Stevens-Haas added a commit to dynamicslab/pysindy that referenced this pull request May 29, 2024
Arviz uses scipy.signal.gaussian, which was removed in 1.13.  Most recent
arviz uses scipy.signal.windows.gaussian

scikit-learn 1.5.0 contained a regression
(scikit-learn/scikit-learn#28352)
that has been fixed in
scikit-learn/scikit-learn#29078
Jacob-Stevens-Haas added a commit to dynamicslab/pysindy that referenced this pull request May 29, 2024
* BLD: Fix broken versions

Arviz uses scipy.signal.gaussian, which was removed in 1.13.  Most recent
arviz uses scipy.signal.windows.gaussian

scikit-learn 1.5.0 contained a regression
(scikit-learn/scikit-learn#28352)
that has been fixed in
scikit-learn/scikit-learn#29078

* BLD: Chasing errors, limit scipy/arviz in SBR

To test the notebooks, need to install SBR extras, which included a version
of arviz that isn't available on 3.9.  Earlier version works, but restricts
scipy version
@lesteve lesteve deleted the gridsearch-1.5-regression branch May 30, 2024 05:47
jeremiedbb pushed a commit to jeremiedbb/scikit-learn that referenced this pull request Jul 2, 2024
@jeremiedbb jeremiedbb mentioned this pull request Jul 2, 2024
11 tasks
jeremiedbb pushed a commit that referenced this pull request Jul 2, 2024
twhsu-stanley pushed a commit to twhsu-stanley/pysindy that referenced this pull request Oct 29, 2024
* BLD: Fix broken versions

Arviz uses scipy.signal.gaussian, which was removed in 1.13.  Most recent
arviz uses scipy.signal.windows.gaussian

scikit-learn 1.5.0 contained a regression
(scikit-learn/scikit-learn#28352)
that has been fixed in
scikit-learn/scikit-learn#29078

* BLD: Chasing errors, limit scipy/arviz in SBR

To test the notebooks, need to install SBR extras, which included a version
of arviz that isn't available on 3.9.  Earlier version works, but restricts
scipy version
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module:model_selection To backport PR merged in master that need a backport to a release branch defined based on the milestone.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GridSearchCV with custom estimator and nested Parameter Grids raises ValueError in scikit-learn 1.5.0
4 participants