[go: up one dir, main page]
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.

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

FIX handle dataframe in ARDRegression when predict requests std. dev. #28377

Merged
merged 6 commits into from
Feb 9, 2024
Merged

FIX handle dataframe in ARDRegression when predict requests std. dev. #28377

merged 6 commits into from
Feb 9, 2024

Conversation

eddiebergman
Copy link
Contributor

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Only change was just to use _safe_indexing.

Any other comments?

  • I added a test for this, not sure if it's the best test.
  • Should this bug fix be documented in the Changelog?
  • I checked BayesianRidge which seems to be tightly related to ArdRegression in testing. The problem doesn't exist there as it doesn't use indexing.

@eddiebergman eddiebergman mentioned this pull request Feb 7, 2024
Closed
@github-actions GitHub Actions
Copy link
github-actions bot commented Feb 7, 2024 •
edited
Loading

✔️ Linting Passed

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

Generated for commit: f53ae89. Link to the linter CI: here

@eddiebergman
Copy link
Contributor Author

I checked the failing CI logs and it seems that some CI does not have pandas installed. Is there some other check that should be done. This is a very pandas/numpy specific issue so I figure it should be imported and used.

https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=63614&view=logs&j=689a1c8f-ff4e-5689-1a1a-6fa551ae9eba&t=0b7e60d2-0e3c-59af-8129-1150b3e7bf0c&l=696

glemaitre
glemaitre reviewed Feb 8, 2024
View reviewed changes
Copy link
Member
@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

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

You will need to add an entry in the changelog doc/whats_new/v1.4.rst (under 1.4.1) to acknowledge the fix.

sklearn/linear_model/tests/test_bayes.py Outdated Show resolved Hide resolved
sklearn/linear_model/tests/test_bayes.py Outdated Show resolved Hide resolved
sklearn/linear_model/tests/test_bayes.py Outdated Show resolved Hide resolved
sklearn/linear_model/tests/test_bayes.py Outdated Show resolved Hide resolved
@glemaitre glemaitre changed the title fix: Use _safe_index to index X for numpy/pandas in ArdRegression FIX handle dataframe in ARDRegression when prediction request std. dev. Feb 8, 2024
@glemaitre glemaitre changed the title FIX handle dataframe in ARDRegression when prediction request std. dev. FIX handle dataframe in ARDRegression when predict requests std. dev. Feb 8, 2024
glemaitre
glemaitre reviewed Feb 8, 2024
View reviewed changes
doc/whats_new/v1.4.rst Outdated Show resolved Hide resolved
glemaitre
glemaitre approved these changes Feb 8, 2024
View reviewed changes
Copy link
Member
@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM. Thanks @eddiebergman

@glemaitre glemaitre added this to the 1.4.1 milestone Feb 8, 2024
jeremiedbb
jeremiedbb approved these changes Feb 9, 2024
View reviewed changes
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 @eddiebergman

@jeremiedbb jeremiedbb merged commit 9c29816 into scikit-learn:main Feb 9, 2024
30 checks passed
glemaitre added a commit to glemaitre/scikit-learn that referenced this pull request Feb 10, 2024
@eddiebergman @glemaitre
FIX handle dataframe in ARDRegression when predict requests std. dev. (…
4bcf134 
…scikit-learn#28377)

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
LeoGrin pushed a commit to LeoGrin/scikit-learn that referenced this pull request Feb 12, 2024
@eddiebergman @glemaitre @LeoGrin
FIX handle dataframe in ARDRegression when predict requests std. dev. (…
636e571 
…scikit-learn#28377)

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
glemaitre added a commit to glemaitre/scikit-learn that referenced this pull request Feb 13, 2024
@eddiebergman @glemaitre
FIX handle dataframe in ARDRegression when predict requests std. dev. (…
e8053cd 
…scikit-learn#28377)

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
glemaitre added a commit that referenced this pull request Feb 13, 2024
@eddiebergman @glemaitre
FIX handle dataframe in ARDRegression when predict requests std. dev. (…
12954ab 
…#28377)

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@glemaitre glemaitre glemaitre approved these changes

@jeremiedbb jeremiedbb jeremiedbb approved these changes

Assignees
No one assigned
Labels
module:linear_model
Projects
None yet
Milestone
1.4.1
Development

Successfully merging this pull request may close these issues.

ARDRegressor variance prediction fails on X: pd.DataFrame
3 participants
@eddiebergman @glemaitre @jeremiedbb