FIX Remove validation in FeatureHasher's __init__ #21573
Conversation
| with pytest.raises(ValueError): | ||
| FeatureHasher(input_type="gobbledygook") | ||
| FeatureHasher(input_type="gobbledygook").transform(raw_X) |
There was a problem hiding this comment.
To make sure that FeatureHasher raises during transform and not __init__:
h = FeatureHasher(input_type="gobbledygook")
with pytest.raises(ValueError):
h.transform(raw_X)Same comment applies to the changes below.
There was a problem hiding this comment.
Thank you for your comment. We have changed the test accordingly.
There was a problem hiding this comment.
Small nit about the whats new, otherwise LGTM!
doc/whats_new/v1.1.rst
Outdated
| - |Fix| :class:`feature_extraction.FeatureHasher` now validates input parameters in `transform` instead of `__init__`. | ||
| :pr:`21573` by :user:`Hannah Bohle <hhnnhh>` and :user:`Maren Westermann <marenwestermann>`. |
There was a problem hiding this comment.
To be consistent with the rest of the file, we can wrap the text here:
- |Fix| :class:`feature_extraction.FeatureHasher` now validates input parameters
in `transform` instead of `__init__`. :pr:`21573` by
:user:`Hannah Bohle <hhnnhh>` and :user:`Maren Westermann <marenwestermann>`.|
@hhnnhh @marenwestermann Hi all, Some of us will be online on the scikit-learn discord Monday December 6 at First time and occasional contributors are welcome to join us to The focus of these office hour sessions is to answer questions about We can also try to assist you into crafting minimal reproduction cases Please note, our Code of Conduct applies: |
| @@ -114,14 +114,20 @@ def test_hash_empty_input(): | |||
|
|
|||
|
|
|||
| def test_hasher_invalid_input(): | |||
| raw_X = [[], (), iter(range(0))] | |||
|
|
|||
| h = FeatureHasher(input_type="gobbledygook") | |||
There was a problem hiding this comment.
| h = FeatureHasher(input_type="gobbledygook") | |
| feature_hasher = FeatureHasher(input_type="gobbledygook") |
There was a problem hiding this comment.
and then reusing this variable afterwards.
There was a problem hiding this comment.
Thank you @glemaitre for your suggestions. We changed the variable name accordingly.
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
…into featurehasher
|
LGTM. Thanks @hhnnhh @marenwestermann |
Reference Issues/PRs
Addresses #21406
What does this implement/fix? Explain your changes.
Removes _validate_params from __init__
Any other comments?
#DataUmbrella