Skip to content

FIX mutual_info_regression when X is of integer dtype #26748

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 4 commits into from
Jul 3, 2023

Conversation

Charlie-XIAO
Copy link
Contributor

@Charlie-XIAO Charlie-XIAO commented Jul 2, 2023

Reference Issues/PRs

Fixes #26696.

What does this implement/fix? Explain your changes.

This PR converts X to float64 dtype before calling scale to avoid rounding to integers.

Btw I reordered the changelog sections according to the 1.3 version and alphabetical order. If not desired I can revert.

@github-actions
Copy link

github-actions bot commented Jul 2, 2023

✔️ Linting Passed

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

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

Copy link
Member

@betatim betatim 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 working on this. Looks reasonable to me.

To keep the review simple and sticking to the "One PR for one topic" I'd propose to not make unrelated changes to the change log.

@Charlie-XIAO
Copy link
Contributor Author

Charlie-XIAO commented Jul 3, 2023

Sure, I will revert the changelog changes and open another PR for it later.

Copy link
Contributor

@Micky774 Micky774 left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for the fix 😄

@Micky774 Micky774 merged commit 1499550 into scikit-learn:main Jul 3, 2023
@Charlie-XIAO Charlie-XIAO deleted the mi-reg-X-int branch July 4, 2023 01:37
punndcoder28 pushed a commit to punndcoder28/scikit-learn that referenced this pull request Jul 29, 2023
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Sep 18, 2023
REDVM pushed a commit to REDVM/scikit-learn that referenced this pull request Nov 16, 2023
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.

mutual_info_regression misbehaves when X is integer-typed
3 participants