FIX Fixes check_array for pd.NA in a series #25080
Conversation
| assert_allclose(X_out, [1, 2, np.nan]) | ||
|
|
||
| X_out = check_array( | ||
| X_int64, force_all_finite=False, ensure_2d=False, dtype=np.float32 |
There was a problem hiding this comment.
For my education: what does this test compared to the previous test? I was expecting one of the assert statements after this to check that the dtype of the return value is float32 or some other intricacy related to float32s.
There was a problem hiding this comment.
Thank you for noticing this! I intended to assert the dtype here too. The PR was updated to include them: 692cfc8 (#25080)
| @@ -777,6 +777,13 @@ def check_array( | |||
| if all(isinstance(dtype_iter, np.dtype) for dtype_iter in dtypes_orig): | |||
| dtype_orig = np.result_type(*dtypes_orig) | |||
|
|
|||
| elif hasattr(array, "iloc") and hasattr(array, "dtype"): | |||
There was a problem hiding this comment.
For my education: why not use isinstance() instead of "guessing" that this is a pd.Series based on the presence of attributes?
There was a problem hiding this comment.
isinstance(..., pd.Series) requires to import pandas which may not be installed since it's not a dependency of scikit-learn
There was a problem hiding this comment.
Makes sense, there are a few parts (outside of examples and benchmarks) where the try: import pandas ... except ImportError: pass pattern is used (for example in _param_validation.py). Maybe worth pondering some kind of consolidation, but for a new issue. Or maybe people already pondered it in the past.
There was a problem hiding this comment.
For the examples and benchmark, we are happy to depend on pandas (it is indeed documented). The idea is to not depend on pandas in the code itself.
In the parameter validation, we use pd because this is the only way to handle the pd.NA beast. We, therefore, try to import it and if pandas is not installed, we know that pd.NA will not be available in anyway.
doc/whats_new/v1.2.rst
Outdated
| @@ -2,6 +2,21 @@ | |||
|
|
|||
| .. currentmodule:: sklearn | |||
|
|
|||
| Version 1.2.1 | |||
There was a problem hiding this comment.
| Version 1.2.1 | |
| .. _changes_1_2_1: | |
| Version 1.2.1 |
There was a problem hiding this comment.
What is the reason for releasing the code with a bug in 1.2.0 since this is only a release? We could directly backport for the next release?
There was a problem hiding this comment.
The changelog entry has been moved to 1.2.
|
Hi folks, I'm not sure if this is the same bug or different but I'm still getting this both in Colab and in Conda: File ~\anaconda3\lib\site-packages\sklearn\utils\validation.py:110, in _assert_all_finite(X, allow_nan, msg_dtype, estimator_name, input_name) AttributeError: 'bool' object has no attribute 'any' |
|
I have just reinstalled sklearn in Conda to no avail. |
Reference Issues/PRs
Fixes #25078
What does this implement/fix? Explain your changes.
This PR updates
check_arrayto properly raise the correct error and casts pandas Series withpd.NA. Note thatcheck_arrayalready has the correct behavior with DataFrames which is tested here:scikit-learn/sklearn/utils/tests/test_validation.py
Line 421 in f2f3b3c