Skip to content

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

Merged
jeremiedbb merged 6 commits into
scikit-learn:mainfrom
eddiebergman:fix-28310
Feb 9, 2024
Merged

jeremiedbb merged 6 commits into
scikit-learn:mainfrom
eddiebergman:fix-28310

Conversation

@eddiebergman

Copy link
Copy Markdown
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.

@github-actions

github-actions Bot commented Feb 7, 2024 •

Copy link
Copy Markdown

✔️ 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
Copy Markdown
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 left a comment

Copy link
Copy Markdown
Member

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.

Comment thread sklearn/linear_model/tests/test_bayes.py Outdated
Comment thread sklearn/linear_model/tests/test_bayes.py Outdated
Comment thread sklearn/linear_model/tests/test_bayes.py Outdated
Comment thread sklearn/linear_model/tests/test_bayes.py Outdated
@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
Comment thread doc/whats_new/v1.4.rst Outdated

@glemaitre glemaitre left a comment

Copy link
Copy Markdown
Member

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 left a comment

Copy link
Copy Markdown
Member

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
glemaitre added a commit to glemaitre/scikit-learn that referenced this pull request Feb 10, 2024
LeoGrin pushed a commit to LeoGrin/scikit-learn that referenced this pull request Feb 12, 2024
glemaitre added a commit to glemaitre/scikit-learn that referenced this pull request Feb 13, 2024
glemaitre added a commit that referenced this pull request Feb 13, 2024
…#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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ARDRegressor variance prediction fails on X: pd.DataFrame

3 participants