ENH Adds pandas IntegerArray support to check_array by thomasjpfan · Pull Request #16508 · scikit-learn/scikit-learn

thomasjpfan · 2020-02-21T00:12:40Z

Reference Issues/PRs

Resolves #16498

What does this implement/fix? Explain your changes.

Uses the name of the dtype to check if the dtype is a pandas IntegerArray.

Any other comments?

The __array__ interface for panda's IntegerArray results in a object:

import pandas as pd
X = pd.Series([1, 2, pd.NA])

np.asarray(X)
# array([1, 2, <NA>], dtype=object)

np.asarray(X, dtype=float)
# ValueError

np.asarray(X.astype(float))
# array([ 1.,  2., nan])

adrinjalali · 2020-04-21T14:53:18Z

@TomAugspurger could you please maybe have a look at this one?

TomAugspurger · 2020-04-21T15:29:00Z

Looks reasonable.

Just to confirm: the desired behavior is to coerce NA to NaN when force_all_finite=True? You're comfortable with the precision loss for large integers that comes with that?

TomAugspurger · 2020-04-21T15:28:06Z

+                    if isinstance(dtype_iter, (Int8Dtype, Int16Dtype,
+                                               Int32Dtype, Int64Dtype)):


We also have unsigned variants. Is that relevant anywhere in check_array?

TomAugspurger · 2020-04-22T18:38:34Z

+        # convert dataframe with Integer extension arrays with floats
+        array = array.astype(None)


I suspect that IntegerArray.astype(None) returning a float64 ndarray isn't deliberate, right @jorisvandenbossche?.

I don't think that is deliberate, indeed (I would say that's a bug .. although it seems consistent with numpy where np.dtype(None) is float64).

thomasjpfan · 2020-04-22T19:02:51Z

Just to confirm: the desired behavior is to coerce NA to NaN when force_all_finite=True?

It is to corece when allow_all_finite is False or 'allow-nan'.

You're comfortable with the precision loss for large integers that comes with that?

It is a compromise to trying to push a extension arrays into a homogeneous numpy array, while making it easier for uses to use pandas + scikit-learn. I am slightly uncomfortable with this and am open to an another approach.

This PR is using the following to cohere:

df = pd.DataFrame(
    {'a': pd.Series([1, 2, 3, None], dtype="UInt8"),
     'b': pd.Series([1, 4, 5, 10], dtype="int")})

df.dtypes
# a    UInt8
# b    int64
# dtype: object

# converst pd.NA to np.nan
df_other = df.astype(None)

df_other.dtype
# a    float64
# b    float64
# dtype: object

which allows us to call asarray and get a numpy array. I do not think this is documented behavior. I can try another approach if this implementation is not officially support by pandas.

TomAugspurger · 2020-04-22T19:23:44Z


+    if has_pd_interger_array:
+        # If there are any pandas integer extension arrays,
+        array = array.astype(None)


I think .astype(np.dtype('float64')) would be best (assuming you always want 64-bit here).

Or to_numpy()? That's even more explicit (and certainly given that astype behaviour might still change, eg to always return new ExtensionArray), although only available for more recent versions? (which might not be a problem, since this is still experimental anyway)

Looking at the actual code now, array here will typically be a DataFrame (in case of pandas), and it seemt that for DataFrame.to_numpy we didn't add the na_value keyword .. (that's a pity, as I think df.to_numpy(dtype='float64', na_value=np.nan) would have been the most robust going forward).

jorisvandenbossche · 2020-04-22T19:43:35Z

You're comfortable with the precision loss for large integers that comes with that?

Note that you can already have this problem with dataframes with the standard numpy dtypes if you have eg a mix of float and int columns (which will all get converted to a single float array)

adrinjalali

codecov complaints seem unrelated. LGTM, thanks @thomasjpfan

jnothman

Add to change log, please.

What's the story with imputers?

jnothman · 2020-04-27T14:18:01Z

What's the story with imputers?

ogrisel

LGTM besides the following comments.

Please also update the docstring of force_all_finite to explicitly mention that pd.NA is accepted and will be converted as np.nan values in a np.float64 arrays (don't forget the versionchanged directive).

ogrisel · 2020-04-27T14:18:57Z

+                                               Int32Dtype, Int64Dtype,
+                                               UInt8Dtype, UInt16Dtype,
+                                               UInt32Dtype, UInt64Dtype)):
+                        has_pd_interger_array = True


Suggested change

has_pd_interger_array = True

has_pd_integer_array = True

ogrisel · 2020-04-27T14:19:21Z

        if all(isinstance(dtype, np.dtype) for dtype in dtypes_orig):
            dtype_orig = np.result_type(*dtypes_orig)

+        if has_pd_interger_array:


Suggested change

if has_pd_interger_array:

if has_pd_integer_array:

ogrisel · 2020-04-27T14:19:42Z

                    "It will be converted to a dense numpy array."
                )

+        has_pd_interger_array = False


Suggested change

has_pd_interger_array = False

has_pd_integer_array = False

ogrisel · 2020-04-27T14:29:25Z

  :pr:`16728` by `Thomas Fan`_.

+- |Enhancement| :func:`utils.validation.check_array` supports pandas
+  IntegerArray. :pr:`16508` by `Thomas Fan`_.


Maybe this could be a bit more descriptive: utils.validation.check_array accepts pandas IntegerArray when force_all_finite is set to False or allow-nan in which case the data is converted to floating point values with pd.NA values replaced by np.nan.

Maybe you could also mention that, as a consequence, all sklearn.preprocessing transformers (also known as "scalers") that accept numeric inputs with missing values represented as np.nan now also accept being directly fed with pandas dataframes with pandas.Int* or pandas.UInt*- typed columns that use pandas.NA as the missing value marker.

ogrisel · 2020-04-27T14:35:53Z

What's the story with imputers?

They might work as a result of this PR but we need to update their own tests and the changelog should also mention them.

I believe the HistGradientBoosting* estimators should now also accept dataframes with IntegerArray columns.

ogrisel

A few more comments:

the docstring of other functions method that expose the force_all_finite option in the public API might also need to be updated similarly:

$ git grep "force_all_finite :"
sklearn/metrics/pairwise.py:    force_all_finite : boolean or 'allow-nan', (default=True)
sklearn/metrics/pairwise.py:    force_all_finite : boolean or 'allow-nan', (default=True)
sklearn/utils/validation.py:    force_all_finite : boolean or 'allow-nan', (default=True)
sklearn/utils/validation.py:    force_all_finite : boolean or 'allow-nan', (default=True)
sklearn/utils/validation.py:    force_all_finite : boolean or 'allow-nan', (default=True)
sklearn/utils/validation.py:    force_all_finite : boolean or 'allow-nan', (default=True)

we need to extend the test to test for the resulting dtype of this conversion:

thomasjpfan · 2020-04-27T14:52:06Z

Added tests to sklearn/impute/tests/test_common.py to test the new functionality with imputers. This can be a little strange because missing_value=np.nan, but the missing value of the input is pd.Na, because we are converting them to np.nan.

ogrisel · 2020-04-27T15:36:14Z

Added tests to sklearn/impute/tests/test_common.py to test the new functionality with imputers. This can be a little strange because missing_value=np.nan, but the missing value of the input is pd.Na, because we are converting them to np.nan.

Maybe we should allow the users to pass missing_value=pd.Na in imputers? Or maybe we should just mention that pd.NA is automatically converted to np.nan in the docstring of the missing_value parameter?

ogrisel · 2020-04-28T12:09:29Z

There are new sphinx warnings in this PR:

Sphinx Warnings in affected files

    sklearn/impute/_base.py:docstring of sklearn.impute.MissingIndicator:15: WARNING: Inline interpreted text or phrase reference start-string without end-string.
    sklearn/impute/_base.py:docstring of sklearn.impute.SimpleImputer:13: WARNING: Inline interpreted text or phrase reference start-string without end-string.
    sklearn/impute/_iterative.py:docstring of sklearn.impute.IterativeImputer:30: WARNING: Inline interpreted text or phrase reference start-string without end-string.
    sklearn/impute/_knn.py:docstring of sklearn.impute.KNNImputer:15: WARNING: Inline interpreted text or phrase reference start-string without end-string.

ogrisel

Typo that breaks sphinx:

jorisvandenbossche · 2020-04-28T12:25:50Z

  for each feature. :pr:`16403` by :user:`Narendra Mukherjee <narendramukherjee>`.

+- |Enhancement| :class:`impute.SimpleImputer`, :class:`impute.KNNImputer`, and
+  :class:`impute.SimpleImputer` accepts pandas IntegerArray with nan values.


Suggested change

:class:`impute.SimpleImputer` accepts pandas IntegerArray with nan values.

:class:`impute.SimpleImputer` accepts pandas' nullable integer dtype with missing values.

Just a suggestion, but in general we try to speak about the dtype, as "IntegerArray" is only used under the hood and not directly visible to most users (unless you access the underlying values of a single column), and thus not necessarily a term that users are familiar with

(but it makes it more verbose, though, ..)

@jorisvandenbossche that's a good point. There are other occurrence of the "IntegerArray" in this PR. I think we should covert them all to "pandas' nullable integer dtype/values/column/dataframe" depending on the context.

@thomasjpfan if you agree with this proposal, I can give it a shot.

I updated the PR with removing the "IntegerArray" wording in place of "pandas' nullable ..."

jorisvandenbossche · 2020-04-28T12:29:13Z

General comment: although integer columns will certainly be the most common use case for scikit-learn, pd.NA is not only used for nullable integer columns in pandas, but also for the dedicated string data type and for the nullable boolean dtype. So we might want to consider more general handling of this not specific to integers in the future.

ogrisel · 2020-04-28T12:32:31Z

So we might want to consider more general handling of this not specific to integers in the future.

In particular for categorical encoders with missing values but this is technically a very different and complementary change, I believe.

ogrisel · 2020-04-28T17:00:24Z

Merged!

thomasjpfan · 2020-04-28T17:03:44Z

#17010

thomasjpfan added 2 commits February 20, 2020 18:14

ENH Adds support for pandas IntegerArray

b936ffb

DOC Adds comment regarding dtype

625eea2

rth reviewed Feb 21, 2020

View reviewed changes

Comment thread sklearn/preprocessing/tests/test_common.py Outdated

Comment thread sklearn/utils/tests/test_validation.py Outdated

thomasjpfan added 3 commits February 21, 2020 11:59

ENH Checks for type explicity

371d16e

CLN Address comments

da39b41

Merge remote-tracking branch 'upstream/master' into pandas_pd_na_support

8bd32d8

github-actions Bot added module:preprocessing module:utils labels Mar 2, 2020

TomAugspurger reviewed Apr 21, 2020

View reviewed changes

thomasjpfan added 2 commits April 22, 2020 12:43

Merge remote-tracking branch 'upstream/master' into pandas_pd_na_support

f1d0a3e

CLN Adds support for unsigned

50a0de9

TomAugspurger reviewed Apr 22, 2020

View reviewed changes

CLN Clean up unneeded code

0f43b16

TomAugspurger reviewed Apr 22, 2020

View reviewed changes

thomasjpfan added 2 commits April 22, 2020 17:13

CLN Address comments

a634d3a

CLN Slightly nicer

cc21a5f

adrinjalali approved these changes Apr 23, 2020

View reviewed changes

Merge remote-tracking branch 'upstream/master' into pandas_pd_na_support

5ef3985

jnothman reviewed Apr 27, 2020

View reviewed changes

Comment thread sklearn/utils/validation.py Outdated

Comment thread sklearn/utils/tests/test_validation.py Outdated

thomasjpfan added 4 commits April 27, 2020 09:57

DOC Adds whats new

c6273a5

TST Adds explicit checking

679c08e

CLN Suggestion

0318eb9

Merge remote-tracking branch 'upstream/master' into pandas_pd_na_support

752ed5a

ogrisel approved these changes Apr 27, 2020

View reviewed changes

ogrisel reviewed Apr 27, 2020

View reviewed changes

Comment thread sklearn/utils/tests/test_validation.py Outdated

CLN Adds test for imputers

39dc7a6

jnothman approved these changes Apr 27, 2020

View reviewed changes

thomasjpfan added 2 commits April 27, 2020 10:55

TST Adds test for dtypes

c861463

ENH Better dtype checking

8ff73e4

thomasjpfan added 2 commits April 27, 2020 12:43

DOC Adds note regarding pandas integerarray

690cfdb

TST Fix

77bf3b5

jnothman approved these changes Apr 28, 2020

View reviewed changes

Merge branch 'master' into pandas_pd_na_support

38873e6

ogrisel reviewed Apr 28, 2020

View reviewed changes

Comment thread sklearn/impute/_base.py Outdated

Comment thread sklearn/impute/_iterative.py Outdated

Comment thread sklearn/impute/_knn.py Outdated

ogrisel reviewed Apr 28, 2020

View reviewed changes

Comment thread sklearn/impute/_base.py Outdated

Apply suggestions from code review

e2b699c

jorisvandenbossche reviewed Apr 28, 2020

View reviewed changes

thomasjpfan and others added 2 commits April 28, 2020 11:14

DOC Update to pands nullable integer dtype with missing values

09d69b5

Merge branch 'master' into pandas_pd_na_support

774eaba

ogrisel merged commit acbe13c into scikit-learn:master Apr 28, 2020

adrinjalali pushed a commit that referenced this pull request Apr 30, 2020

ENH Adds pandas IntegerArray support to check_array (#16508)

7c24d0a

gio8tisu pushed a commit to gio8tisu/scikit-learn that referenced this pull request May 15, 2020

ENH Adds pandas IntegerArray support to check_array (scikit-learn#16508)

6d383d0

TomAugspurger mentioned this pull request May 27, 2020

Missing Data data-apis/dataframe-api#9

Open

viclafargue pushed a commit to viclafargue/scikit-learn that referenced this pull request Jun 26, 2020

ENH Adds pandas IntegerArray support to check_array (scikit-learn#16508)

04eaa13

		if isinstance(dtype_iter, (Int8Dtype, Int16Dtype,
		Int32Dtype, Int64Dtype)):

		# convert dataframe with Integer extension arrays with floats
		array = array.astype(None)

	:class:`impute.SimpleImputer` accepts pandas IntegerArray with nan values.
	:class:`impute.SimpleImputer` accepts pandas' nullable integer dtype with missing values.

Conversation

thomasjpfan commented Feb 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Any other comments?

Uh oh!

Uh oh!

Uh oh!

adrinjalali commented Apr 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TomAugspurger commented Apr 21, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TomAugspurger Apr 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thomasjpfan commented Apr 22, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jorisvandenbossche commented Apr 22, 2020

Uh oh!

adrinjalali left a comment

Choose a reason for hiding this comment

Uh oh!

jnothman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jnothman commented Apr 27, 2020

Uh oh!

ogrisel left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ogrisel Apr 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ogrisel commented Apr 27, 2020

Uh oh!

ogrisel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

thomasjpfan commented Apr 27, 2020

Uh oh!

ogrisel commented Apr 27, 2020

Uh oh!

ogrisel commented Apr 28, 2020

Uh oh!

ogrisel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jorisvandenbossche Apr 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thomasjpfan commented Feb 21, 2020 •

edited

Loading

adrinjalali commented Apr 21, 2020 •

edited

Loading

TomAugspurger Apr 22, 2020 •

edited

Loading

ogrisel Apr 27, 2020 •

edited

Loading

jorisvandenbossche Apr 28, 2020 •

edited

Loading