8000 Comparing string to array in _estimate_mi · Issue #13481 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

Comparing string to array in _estimate_mi #13481

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
amueller opened this issue Mar 20, 2019 · 7 comments · Fixed by #13497
Closed

Comparing string to array in _estimate_mi #13481

amueller opened this issue Mar 20, 2019 · 7 comments · Fixed by #13497
Labels
Bug Easy Well-defined and straightforward way to resolve good first issue Easy with clear instructions to resolve help wanted

Comments

@amueller
Copy link
Member

In _estimate_mi there is discrete_features == 'auto' but discrete features can be an array of indices or a boolean mask.
This will error in future versions of numpy.
Also this means we never test this function with discrete features != 'auto', it seems?
@amueller amueller added Bug Easy Well-defined and straightforward way to resolve help wanted good first issue Easy with clear instructions to resolve labels Mar 20, 2019
@hermidalc
Copy link
Contributor

I'll take this

@amueller
Copy link
Member Author

@hermidalc go for it :)

@punkstar25
Copy link

i'm not sure ,but i think user will change the default value if it seem to be array or boolean mask....bcz auto is default value it is not fixed.

@jnothman
Copy link
Member

I haven't understood, @punkstar25

@hermidalc hermidalc mentioned this issue Mar 23, 2019
Merged
@MohammedKhandwawala
Copy link
MohammedKhandwawala commented Mar 26, 2019 •
edited
Loading

@amueller i want to take this issue. I am a beginner.
Also can you elaborate this
"Also this means we never test this function with discrete features != 'auto', it seems?"
What should it return if discrete features is a string and not "auto"

@NicolasHug
Copy link
Member

@MohammedKhandwawala , I'm afraid @hermidalc has already opened #13497 to address this.

@hermidalc
Copy link
Contributor
hermidalc commented Mar 26, 2019 •
edited
Loading

@amueller i want to take this issue. I am a beginner.
Also can you elaborate this
"Also this means we never test this function with discrete features != 'auto', it seems?"
What should it return if discrete features is a string and not "auto"

@MohammedKhandwawala I have a pull request open and am finishing this issue. To answer your question what I believe @amueller meant is to properly test and if necessary raise an exception for discrete_features string cases not equal to 'auto' as well as other invalid parameter values. The updated if/elif statements in the pull request should fix that as well as checking for a str instance before testing == 'auto' which would fail if a numpy array in future versions of numpy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Easy Well-defined and straightforward way to resolve good first issue Easy with clear instructions to resolve help wanted
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix _estimate_mi discrete_features str and value check hermidalc/scikit-learn
6 participants
@jnothman @amueller @hermidalc @NicolasHug @MohammedKhandwawala @punkstar25
0