8000 Use scipy.stats.mstats.mode in SimpleImputer when strategy="most_frequent" · Issue #14325 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

Use scipy.stats.mstats.mode in SimpleImputer when strategy="most_frequent" #14325

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
Closed
qinhanmin2014 opened this issue Jul 13, 2019 · 6 comments
Closed

Use scipy.stats.mstats.mode in SimpleImputer when strategy="most_frequent" #14325

qinhanmin2014 opened this issue Jul 13, 2019 · 6 comments
Labels
help wanted

Comments

@qinhanmin2014
Copy link
Member

scikit-learn/sklearn/impute/_base.py

Lines 341 to 344 in 979c761

# scipy.stats.mstats.mode cannot be used because it will no work
# properly if the first element is masked and if its frequency
# is equal to the frequency of the most frequent valid element
# See https://github.com/scipy/scipy/issues/2636

the fix is included in scipy 0.13.0 (see scipy/scipy@66f71a4), we now require scipy >= 0.17.0
@jcusick13
Copy link
Contributor

If no one's already started, I'd be glad to work on this.

@jcusick13
Copy link
Contributor

@qinhanmin2014, should I be looking to replace the _most_frequent() function with scipy.stats.mstats.mode() or should I instead try to just clean up the code block within strategy == 'most_frequent'?

It looks like the fixed mode() function doesn't entirely replace the behavior of _most_frequent(), so I'm not sure how much of a change you were originally thinking.

@qinhanmin2014
Copy link
Member Author

Just try to simply the code and remove that comment.

@jcusick13 jcusick13 mentioned this issue Jul 18, 2019
Merged
@vinidixit
Copy link
vinidixit commented Jul 25, 2019 •
edited
Loading

@qinhanmin2014 Hi! Are you still looking for help? I'm interested in contributing to this issue. Please let me know in case you could occupy.

@jcusick13
Copy link
Contributor

@vinidixit I've got a PR open now that I need to finish up some benchmarking for. I'm hoping to have it finalized this weekend. I'll reach out to you if I can't make time for it though.

@jnothman jnothman mentioned this issue Dec 8, 2019
Open
@jeremiedbb
Copy link
Member

fixed in #14399. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@qinhanmin2014 @vinidixit @jcusick13 @jeremiedbb
0