Skip to content

[MRG+1] Rework warning in check_array when silent convert string to float #11577

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

Conversation

massich
Copy link
Contributor

@massich massich commented Jul 16, 2018

This PR rewords #10495, and fix the tests since the check_array was called without keywords therefore assert_warns_message(FutureWarning, "....", check_array, X_str, "numeric") was translated to

check_array(X_str, accept_sparse="numeric")

@massich
Copy link
Contributor Author

massich commented Jul 16, 2018

cc: @amueller

@massich
Copy link
Contributor Author

massich commented Jul 16, 2018

Once said that, IMO the proper move would be to add a Deprecation warning so that we can catch early X=[['some', 'random'],['string', 'xxx']]

Any thoughts?
cc: @lesteve

@massich massich changed the title Rework warning in check_array when silent convert string to float [MRG] Rework warning in check_array when silent convert string to float Jul 16, 2018
@massich
Copy link
Contributor Author

massich commented Jul 16, 2018

IMO the test suit should be something like what @lesteve porposed #10495 (comment)

check_array(np.array(['1', '2'], dtype='O'), dtype='numeric') # error
check_array(np.array(['1', '2'], dtype='S1'), dtype='numeric') # error
check_array(np.array(['1', '2'], dtype='U1'), dtype='numeric') # error
check_array(np.array([1, 2], dtype='O'), dtype='numeric') # no error

@GaelVaroquaux GaelVaroquaux changed the title [MRG] Rework warning in check_array when silent convert string to float [MRG+1] Rework warning in check_array when silent convert string to float Jul 17, 2018
Copy link
Member

@GaelVaroquaux GaelVaroquaux left a comment

Choose a reason for hiding this comment

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

LGTM. +1 for merge.

@amueller amueller merged commit 2d7567f into scikit-learn:master Jul 17, 2018
@massich massich deleted the warn_on_string_float_silent_conversion branch July 17, 2018 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants