FIX handle VisibleDeprecationWarning in type_of_target and is_multilabel #18423
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alfaro96
reviewed
Sep 20, 2020
There was a problem hiding this comment.
Hey @OlehKSS,
Thank you for your PR.
I think we should comment why y = np.asarray(y, dtype=object) is required. Maybe something like:
# Specify dtype=object type for ragged arrays because
# NEP 34 disallow inferring these arrays from sequencesMoreover, we should include a TODO note to fix this issue when changing the behaviour to raise a ValueError instead.
99ae8b8 to
4301144
Compare
|
Hi, @alfaro96 |
|
Same as #18414, milestoned as 0.24 in order to update tests with respect to newer versions of numpy. |
glemaitre
reviewed
Oct 28, 2020
sklearn/utils/multiclass.py
Outdated
| @@ -137,8 +137,20 @@ def is_multilabel(y): | |||
| >>> is_multilabel(np.array([[1, 0, 0]])) | |||
| True | |||
| """ | |||
| import warnings | |||
There was a problem hiding this comment.
We should import warnings on the top of the file
glemaitre
reviewed
Oct 28, 2020
| if hasattr(y, '__array__') or isinstance(y, Sequence): | ||
| y = np.asarray(y) | ||
| # TODO: Replace the warning context manager with a try-except statement | ||
| # DeprecationWarning will be replaced by ValueError, see NEP 34 |
There was a problem hiding this comment.
Suggested change
| # DeprecationWarning will be replaced by ValueError, see NEP 34 | |
| # DeprecationWarning will be replaced by ValueError, see NEP 34 | |
| # https://numpy.org/neps/nep-0034-infer-dtype-is-object.html |
glemaitre
reviewed
Oct 28, 2020
sklearn/utils/multiclass.py
Outdated
| @@ -236,6 +248,8 @@ def type_of_target(y): | |||
| >>> type_of_target(np.array([[0, 1], [1, 1]])) | |||
| 'multilabel-indicator' | |||
| """ | |||
| import warnings | |||
glemaitre
reviewed
Oct 28, 2020
| @@ -250,7 +264,16 @@ def type_of_target(y): | |||
| if is_multilabel(y): | |||
| return 'multilabel-indicator' | |||
|
|
|||
| y = np.asarray(y) | |||
| # TODO: Replace the warning context manager with a try-except statement | |||
| # DeprecationWarning will be replaced by ValueError, see NEP 34 | |||
There was a problem hiding this comment.
Suggested change
| # DeprecationWarning will be replaced by ValueError, see NEP 34 | |
| # DeprecationWarning will be replaced by ValueError, see NEP 34 | |
| # https://numpy.org/neps/nep-0034-infer-dtype-is-object.html |
|
@glemaitre I have updated this pull request according to your suggestions |
thomasjpfan
approved these changes
Oct 29, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reference Issues/PRs
Fix #18367
What does this implement/fix? Explain your changes.
Fix
numpy.VisibleDeprecationWarningintype_of_target,is_multilabel.This warning was issued by
numpy.asarraybecause of ragged/jagged sequences that were provided to this function. It was spotted in the following unit tests:Any other comments?
As far as I see, there are several ways to solve it:
numpy.arrayinstead. In that case, only users will see this warningnumpy.asarray(, dtype=object)numpy.asarraycorrectly.I decided to it as in the 3rd list item. Here, I assumed that it'd better let NumPy do this test and then catch that warning as an exception. Not sure whether it is the optimal way to do it. What do you think?