Skip to content
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

ENH: ExtensionArray.fillna #19909

Merged
merged 26 commits into from
Mar 16, 2018
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
74614cb
ENH: ExtensionArray.fillna
TomAugspurger Feb 22, 2018
67a19ba
REF: cast to object
TomAugspurger Feb 27, 2018
280ac94
Revert "REF: cast to object"
TomAugspurger Feb 27, 2018
6705712
Merge remote-tracking branch 'upstream/master' into fu1+fillna
TomAugspurger Feb 28, 2018
69a0f9b
Simpler implementation
TomAugspurger Feb 28, 2018
4d6846b
Support limit
TomAugspurger Feb 28, 2018
f3b81dc
Test backfill with limit
TomAugspurger Feb 28, 2018
70efbb8
BUG: ensure array-like for indexer
TomAugspurger Feb 28, 2018
8fc3851
Refactor tolist
TomAugspurger Mar 1, 2018
39096e5
Test Series[ea].tolist
TomAugspurger Mar 1, 2018
9e44f88
Merge remote-tracking branch 'upstream/master' into fu1+fillna
TomAugspurger Mar 1, 2018
e902f18
Fixed Categorical unwrapping
TomAugspurger Mar 1, 2018
17126e6
updated
TomAugspurger Mar 2, 2018
1106ef2
Back to getvalues list
TomAugspurger Mar 2, 2018
744c381
Simplified
TomAugspurger Mar 2, 2018
9a3fa55
As a method
TomAugspurger Mar 2, 2018
2bc43bc
Merge remote-tracking branch 'upstream/master' into fu1+fillna
TomAugspurger Mar 3, 2018
f22fd7b
Removed tolist fully
TomAugspurger Mar 3, 2018
c26d261
Merge remote-tracking branch 'upstream/master' into fu1+fillna
TomAugspurger Mar 5, 2018
b342efe
Linting
TomAugspurger Mar 5, 2018
a609f48
Merge remote-tracking branch 'upstream/master' into fu1+fillna
TomAugspurger Mar 12, 2018
a35f93c
Just apply to objects
TomAugspurger Mar 12, 2018
3f78dec
Merge remote-tracking branch 'upstream/master' into fu1+fillna
TomAugspurger Mar 12, 2018
1160e15
Linting
TomAugspurger Mar 13, 2018
c9c7a48
Merge remote-tracking branch 'upstream/master' into fu1+fillna
TomAugspurger Mar 14, 2018
05fced6
Merge remote-tracking branch 'upstream/master' into fu1+fillna
TomAugspurger Mar 15, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions pandas/core/arrays/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,70 @@ def isna(self):
"""
raise AbstractMethodError(self)

def fillna(self, value=None, method=None, limit=None):
""" Fill NA/NaN values using the specified method.

Parameters
----------
value : scalar, array-like
Copy link
Member

Choose a reason for hiding this comment

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

value before method ? (order of signature)

If a scalar value is passed it is used to fill all missing values.
Alternatively, an array-like 'value' can be given. It's expected
that the array-like have the same length as 'self'.
method : {'backfill', 'bfill', 'pad', 'ffill', None}, default None
Method to use for filling holes in reindexed Series
pad / ffill: propagate last valid observation forward to next valid
backfill / bfill: use NEXT valid observation to fill gap
limit : int, default None
If method is specified, this is the maximum number of consecutive
NaN values to forward/backward fill. In other words, if there is
a gap with more than this number of consecutive NaNs, it will only
be partially filled. If method is not specified, this is the
maximum number of entries along the entire axis where NaNs will be
filled.

Returns
-------
filled : ExtensionArray with NA/NaN filled
"""
from pandas.api.types import is_scalar
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A putmask type method would be extremely useful here.

I'll see what I can do to simplify this.

Copy link
Member

Choose a reason for hiding this comment

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

How would that be different with array[mask] = values ?

from pandas.util._validators import validate_fillna_kwargs
from pandas.core.missing import pad_1d, backfill_1d
from pandas.core.dtypes.common import _ensure_platform_int
from pandas._libs.tslib import iNaT

value, method = validate_fillna_kwargs(value, method)

mask = self.isna()

if not is_scalar(value):
if len(value) != len(self):
raise ValueError("Length of 'value' does not match. Got ({}) "
" expected {}".format(len(value), len(self)))
value = value[mask]

if mask.any():
if method is not None:
# ffill / bfill
# The basic idea is to create an array of integer positions.
# Internally, we use iNaT and the datetime filling routines
# to avoid floating-point NaN. Once filled, we take on `self`
# to get the actual values.
Copy link
Contributor

Choose a reason for hiding this comment

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

I am still not clear why you are writing another implementation of fill here. We already have one for object dtypes. I would suggest using that one. Yes I get that you want one for the extension classes, but if that is the case then I would replace the algos we have with THIS one. There is no reason to have 2 floating around. Then this will be fully tested and you can look at performance and such.

I don't really have a preference to the these 2 options.

The extension classes implementation MUST be integrated if its in the base class.

func = pad_1d if method == 'pad' else backfill_1d
Copy link
Member

Choose a reason for hiding this comment

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

is method normalized by here? (I mean, can it also be 'ffill'?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, by validate_fillna_kwargs

idx = np.arange(len(self), dtype='int64')
idx[mask] = iNaT
idx = _ensure_platform_int(func(idx, mask=mask,
limit=limit,
dtype='datetime64[ns]'))
idx[idx == iNaT] = -1 # missing value marker for take.
new_values = self.take(idx)
else:
# fill with value
new_values = self.copy()
new_values[mask] = value
else:
new_values = self.copy()
return new_values

# ------------------------------------------------------------------------
# Indexing methods
# ------------------------------------------------------------------------
Expand Down Expand Up @@ -253,6 +317,7 @@ def take(self, indexer, allow_fill=True, fill_value=None):
.. code-block:: python

def take(self, indexer, allow_fill=True, fill_value=None):
indexer = np.asarray(indexer)
mask = indexer == -1
result = self.data.take(indexer)
result[mask] = np.nan # NA for this type
Expand Down
8 changes: 4 additions & 4 deletions pandas/core/arrays/categorical.py
Original file line number Diff line number Diff line change
Expand Up @@ -1587,16 +1587,16 @@ def fillna(self, value=None, method=None, limit=None):

Parameters
----------
method : {'backfill', 'bfill', 'pad', 'ffill', None}, default None
Method to use for filling holes in reindexed Series
pad / ffill: propagate last valid observation forward to next valid
backfill / bfill: use NEXT valid observation to fill gap
value : scalar, dict, Series
If a scalar value is passed it is used to fill all missing values.
Alternatively, a Series or dict can be used to fill in different
values for each index. The value should not be a list. The
value(s) passed should either be in the categories or should be
NaN.
method : {'backfill', 'bfill', 'pad', 'ffill', None}, default None
Method to use for filling holes in reindexed Series
pad / ffill: propagate last valid observation forward to next valid
backfill / bfill: use NEXT valid observation to fill gap
limit : int, default None
(Not implemented yet for Categorical!)
If method is specified, this is the maximum number of consecutive
Expand Down
8 changes: 2 additions & 6 deletions pandas/core/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@

from pandas.core.dtypes.missing import isna
from pandas.core.dtypes.generic import ABCDataFrame, ABCSeries, ABCIndexClass
from pandas.core.dtypes.cast import tolist
from pandas.core.dtypes.common import (
is_object_dtype,
is_list_like,
is_scalar,
is_datetimelike,
is_extension_type,
is_extension_array_dtype)

Expand Down Expand Up @@ -826,11 +826,7 @@ def tolist(self):
--------
numpy.ndarray.tolist
"""

if is_datetimelike(self):
return [com._maybe_box_datetimelike(x) for x in self._values]
else:
return self._values.tolist()
return tolist(self._values)

def __iter__(self):
"""
Expand Down
32 changes: 31 additions & 1 deletion pandas/core/dtypes/cast.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
from .common import (_ensure_object, is_bool, is_integer, is_float,
is_complex, is_datetimetz, is_categorical_dtype,
is_datetimelike,
is_extension_type, is_object_dtype,
is_extension_type, is_extension_array_dtype,
is_object_dtype,
is_datetime64tz_dtype, is_datetime64_dtype,
is_datetime64_ns_dtype,
is_timedelta64_dtype, is_timedelta64_ns_dtype,
Expand Down Expand Up @@ -1222,3 +1223,32 @@ def construct_1d_object_array_from_listlike(values):
result = np.empty(len(values), dtype='object')
result[:] = values
return result


def tolist(values):
"""Convert an array-like to a list of scalar types.

Parameters
----------
values : ndarray or ExtensionArray

Returns
-------
list
Each element of the list is a Python scalar (stor, int float)
or a pandas scalar (Timestamp / Timedelta / Interval / Period) or
the scalar type for 3rd party Extension Arrays.

See Also
--------
Series.tolist
numpy.ndarray.tolist
"""
from pandas.core.common import _maybe_box_datetimelike

if is_datetimelike(values):
return [_maybe_box_datetimelike(x) for x in values]
elif is_extension_array_dtype(values):
return list(values)
else:
return values.tolist()
38 changes: 17 additions & 21 deletions pandas/core/internals.py
Original file line number Diff line number Diff line change
Expand Up @@ -1963,6 +1963,23 @@ def concat_same_type(self, to_concat, placement=None):
return self.make_block_same_class(values, ndim=self.ndim,
placement=placement)

def fillna(self, value, limit=None, inplace=False, downcast=None,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was moved from Categorical.fillna with changes

1.) Removed check for limit, since that's done in values.fillna
2.) Removed _try_coerce_result
For CategoricalBlock, this was unnecessary since
Categorical.fillna always returns a Categorical
3.) Used make_block_same_class
This limits ExtensionArray.fillna to not change the type
of the array / block, which I think is a good thing.

mgr=None):
values = self.values if inplace else self.values.copy()
values = values.fillna(value=value, limit=limit)
return [self.make_block_same_class(values=values,
placement=self.mgr_locs,
ndim=self.ndim)]

def interpolate(self, method='pad', axis=0, inplace=False, limit=None,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was just a move from CategoricalBlock to ExtensionBlock, no changes.

fill_value=None, **kwargs):

values = self.values if inplace else self.values.copy()
return self.make_block_same_class(
values=values.fillna(value=fill_value, method=method,
limit=limit),
placement=self.mgr_locs)


class NumericBlock(Block):
__slots__ = ()
Expand Down Expand Up @@ -2522,27 +2539,6 @@ def _try_coerce_result(self, result):

return result

def fillna(self, value, limit=None, inplace=False, downcast=None,
mgr=None):
# we may need to upcast our fill to match our dtype
if limit is not None:
raise NotImplementedError("specifying a limit for 'fillna' has "
"not been implemented yet")

values = self.values if inplace else self.values.copy()
values = self._try_coerce_result(values.fillna(value=value,
limit=limit))
return [self.make_block(values=values)]

def interpolate(self, method='pad', axis=0, inplace=False, limit=None,
fill_value=None, **kwargs):

values = self.values if inplace else self.values.copy()
return self.make_block_same_class(
values=values.fillna(fill_value=fill_value, method=method,
limit=limit),
placement=self.mgr_locs)

def shift(self, periods, axis=0, mgr=None):
return self.make_block_same_class(values=self.values.shift(periods),
placement=self.mgr_locs)
Expand Down
15 changes: 14 additions & 1 deletion pandas/tests/dtypes/test_cast.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
maybe_convert_scalar,
find_common_type,
construct_1d_object_array_from_listlike,
construct_1d_arraylike_from_scalar)
construct_1d_arraylike_from_scalar,
tolist)
from pandas.core.dtypes.dtypes import (
CategoricalDtype,
DatetimeTZDtype,
Expand Down Expand Up @@ -324,6 +325,18 @@ def test_maybe_convert_objects_copy(self):
out = maybe_convert_objects(values, copy=True)
assert values is not out

@pytest.mark.parametrize('values, expected', [
(pd.date_range('2017', periods=1), [pd.Timestamp('2017')]),
(pd.period_range('2017', periods=1, freq='D'),
[pd.Period('2017', freq='D')]),
(pd.timedelta_range(0, periods=1), [pd.Timedelta('0')]),
(pd.interval_range(0, periods=1), [pd.Interval(0, 1)]),
(np.array([0, 1]), [0, 1]),
])
def test_tolist(self, values, expected):
result = tolist(values)
assert result == expected


class TestCommonTypes(object):

Expand Down
6 changes: 6 additions & 0 deletions pandas/tests/extension/base/interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import pandas as pd
from pandas.compat import StringIO
from pandas.core.dtypes.cast import tolist
from pandas.core.dtypes.common import is_extension_array_dtype
from pandas.core.dtypes.dtypes import ExtensionDtype

Expand Down Expand Up @@ -53,3 +54,8 @@ def test_is_extension_array_dtype(self, data):
assert is_extension_array_dtype(data.dtype)
assert is_extension_array_dtype(pd.Series(data))
assert isinstance(data.dtype, ExtensionDtype)

def test_tolist(self, data):
result = tolist(data)
Copy link
Member

Choose a reason for hiding this comment

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

maybe rather test Series[extension].tolist() here? to check it is the same as list(data) (instead of testing this internal method)

expected = list(data)
assert result == expected
69 changes: 69 additions & 0 deletions pandas/tests/extension/base/missing.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import numpy as np
import pytest

import pandas as pd
import pandas.util.testing as tm
Expand Down Expand Up @@ -45,3 +46,71 @@ def test_dropna_frame(self, data_missing):
result = df.dropna()
expected = df.iloc[:0]
self.assert_frame_equal(result, expected)

def test_fillna_limit_pad(self, data_missing):
arr = data_missing.take([1, 0, 0, 0, 1])
result = pd.Series(arr).fillna(method='ffill', limit=2)
expected = pd.Series(data_missing.take([1, 1, 1, 0, 1]))
self.assert_series_equal(result, expected)

def test_fillna_limit_backfill(self, data_missing):
arr = data_missing.take([1, 0, 0, 0, 1])
result = pd.Series(arr).fillna(method='backfill', limit=2)
expected = pd.Series(data_missing.take([1, 0, 1, 1, 1]))
self.assert_series_equal(result, expected)

def test_fillna_series(self, data_missing):
fill_value = data_missing[1]
ser = pd.Series(data_missing)

result = ser.fillna(fill_value)
expected = pd.Series(type(data_missing)([fill_value, fill_value]))
self.assert_series_equal(result, expected)

# Fill with a series
result = ser.fillna(expected)
self.assert_series_equal(result, expected)

# Fill with a series not affecting the missing values
result = ser.fillna(ser)
self.assert_series_equal(result, ser)

@pytest.mark.parametrize('method', ['ffill', 'bfill'])
def test_fillna_series_method(self, data_missing, method):
fill_value = data_missing[1]

if method == 'ffill':
data_missing = type(data_missing)(data_missing[::-1])

result = pd.Series(data_missing).fillna(method=method)
expected = pd.Series(type(data_missing)([fill_value, fill_value]))

self.assert_series_equal(result, expected)

def test_fillna_frame(self, data_missing):
fill_value = data_missing[1]

result = pd.DataFrame({
"A": data_missing,
"B": [1, 2]
}).fillna(fill_value)

expected = pd.DataFrame({
"A": type(data_missing)([fill_value, fill_value]),
"B": [1, 2],
})

self.assert_frame_equal(result, expected)

def test_fillna_fill_other(self, data):
result = pd.DataFrame({
"A": data,
"B": [np.nan] * len(data)
}).fillna({"B": 0.0})

expected = pd.DataFrame({
"A": data,
"B": [0.0] * len(result),
})

self.assert_frame_equal(result, expected)
9 changes: 8 additions & 1 deletion pandas/tests/extension/category/test_categorical.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,14 @@ def test_getitem_scalar(self):


class TestMissing(base.BaseMissingTests):
pass

@pytest.mark.skip(reason="Not implemented")
def test_fillna_limit_pad(self):
pass

@pytest.mark.skip(reason="Not implemented")
def test_fillna_limit_backfill(self):
pass


class TestMethods(base.BaseMethodsTests):
Expand Down
1 change: 1 addition & 0 deletions pandas/tests/extension/decimal/array.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ def isna(self):
return np.array([x.is_nan() for x in self.values])

def take(self, indexer, allow_fill=True, fill_value=None):
indexer = np.asarray(indexer)
mask = indexer == -1

indexer = _ensure_platform_int(indexer)
Expand Down
Loading