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

BUG+DEPR: undeprecate item, fix dt64/td64 output type #30175

Merged
merged 10 commits into from
Dec 18, 2019
3 changes: 3 additions & 0 deletions doc/source/whatsnew/v1.0.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,7 @@ Deprecations
- :func:`eval` keyword argument "truediv" is deprecated and will be removed in a future version (:issue:`29812`)
- :meth:`Categorical.take_nd` is deprecated, use :meth:`Categorical.take` instead (:issue:`27745`)
- The parameter ``numeric_only`` of :meth:`Categorical.min` and :meth:`Categorical.max` is deprecated and replaced with ``skipna`` (:issue:`25303`)
- :meth:`Series.item` and :meth:`Index.item` have been _undeprecated_ (:issue:`?????`)
-

.. _whatsnew_1000.prior_deprecations:
Expand Down Expand Up @@ -683,6 +684,8 @@ Datetimelike
- Bug in :meth:`Series.var` failing to raise ``TypeError`` when called with ``timedelta64[ns]`` dtype (:issue:`28289`)
- Bug in :meth:`DatetimeIndex.strftime` and :meth:`Series.dt.strftime` where ``NaT`` was converted to the string ``'NaT'`` instead of ``np.nan`` (:issue:`29578`)
- Bug in :attr:`Timestamp.resolution` being a property instead of a class attribute (:issue:`29910`)
- Bug in :meth:`Series.item` with ``datetime64`` or ``timedelta64`` dtype, :meth:`DatetimeIndex.item`, and :meth:`TimedeltaIndex.item` returning an integer instead of a :class:`Timestamp` or :class:`Timedelta` (:issue:`????`)
-

Timedelta
^^^^^^^^^
Expand Down
18 changes: 9 additions & 9 deletions pandas/core/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from collections import OrderedDict
import textwrap
from typing import Dict, FrozenSet, List, Optional
import warnings

import numpy as np

Expand Down Expand Up @@ -677,19 +676,20 @@ def item(self):
"""
Return the first element of the underlying data as a python scalar.

.. deprecated:: 0.25.0

Returns
-------
scalar
The first element of %(klass)s.

Raises
------
ValueError
If the data is not length-1.
"""
warnings.warn(
"`item` has been deprecated and will be removed in a future version",
FutureWarning,
stacklevel=2,
)
return self.values.item()
if len(self) == 1:
Copy link
Member

Choose a reason for hiding this comment

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

Was adding this condition discussed somewhere? I would have thought just keep existing behaviour

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the bugfix part. dt64, dt64tz, and td64 we're currently incorrectly returning int

Copy link
Member

Choose a reason for hiding this comment

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

Hmm doesn't this break non-DTA though?

>>> type(pd.Series(range(1)).item())
<class 'int'>
>>> type(pd.Series(range(1))[0])
<class 'numpy.int64'>

I thought one of the points of item was to return a Python object (at least in the Numpy world)

Copy link
Member

Choose a reason for hiding this comment

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

^ current behavior

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I think we should keep the behaviour of item to return a python scalar (where possible of course, so for datetime/timedelta it is fine to return a pandas Timestamp/Timedelta I think)

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, will update.

Copy link
Contributor

Choose a reason for hiding this comment

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

Has this been resolved?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the concerns raised by @WillAyd and @jorisvandenbossche have been addressed

return self[0]
else:
raise ValueError("can only convert an array of size 1 to a Python scalar")

@property
def nbytes(self):
Expand Down
15 changes: 1 addition & 14 deletions pandas/core/indexes/period.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from datetime import datetime, timedelta
import warnings
import weakref

import numpy as np
Expand Down Expand Up @@ -890,23 +889,11 @@ def __setstate__(self, state):

def item(self):
"""
Return the first element of the underlying data as a python
scalar

.. deprecated:: 0.25.0

Return the first element of the underlying data as a python scalar.
jbrockmendel marked this conversation as resolved.
Show resolved Hide resolved
"""
warnings.warn(
"`item` has been deprecated and will be removed in a future version",
FutureWarning,
stacklevel=2,
)
# TODO(DatetimeArray): remove
if len(self) == 1:
return self[0]
else:
# TODO: is this still necessary?
# copy numpy's message here because Py26 raises an IndexError
raise ValueError("can only convert an array of size 1 to a Python scalar")

def memory_usage(self, deep=False):
Expand Down
8 changes: 3 additions & 5 deletions pandas/tests/base/test_ops.py
Original file line number Diff line number Diff line change
Expand Up @@ -236,15 +236,13 @@ def test_ndarray_compat_properties(self):
assert not hasattr(o, p)

with pytest.raises(ValueError):
with tm.assert_produces_warning(FutureWarning):
o.item() # len > 1
o.item() # len > 1

assert o.ndim == 1
assert o.size == len(o)

with tm.assert_produces_warning(FutureWarning):
assert Index([1]).item() == 1
assert Series([1]).item() == 1
assert Index([1]).item() == 1
assert Series([1]).item() == 1

def test_value_counts_unique_nunique(self):
for orig in self.objs:
Expand Down
43 changes: 35 additions & 8 deletions pandas/tests/series/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,14 @@
DatetimeIndex,
Index,
Series,
Timedelta,
TimedeltaIndex,
Timestamp,
date_range,
period_range,
timedelta_range,
)
from pandas.core.arrays import PeriodArray
from pandas.core.indexes.datetimes import Timestamp
import pandas.util.testing as tm

import pandas.io.formats.printing as printing
Expand Down Expand Up @@ -398,6 +399,39 @@ def test_numpy_unique(self, datetime_series):
# it works!
np.unique(datetime_series)

def test_item(self):
s = Series([1])
result = s.item()
assert result == 1
assert s.item() == s.iloc[0]

ser = Series([1, 2])
msg = "can only convert an array of size 1"
with pytest.raises(ValueError, match=msg):
ser.item()

dti = pd.date_range("2016-01-01", periods=2)
with pytest.raises(ValueError, match=msg):
dti.item()
with pytest.raises(ValueError, match=msg):
Series(dti).item()

val = dti[:1].item()
assert isinstance(val, Timestamp)
val = Series(dti)[:1].item()
assert isinstance(val, Timestamp)

tdi = dti - dti
with pytest.raises(ValueError, match=msg):
tdi.item()
with pytest.raises(ValueError, match=msg):
Series(tdi).item()

val = tdi[:1].item()
assert isinstance(val, Timedelta)
val = Series(tdi)[:1].item()
assert isinstance(val, Timedelta)

def test_ndarray_compat(self):

# test numpy compat with Series as sub-class of NDFrame
Expand All @@ -414,13 +448,6 @@ def f(x):
expected = tsdf.max()
tm.assert_series_equal(result, expected)

# .item()
with tm.assert_produces_warning(FutureWarning):
s = Series([1])
result = s.item()
assert result == 1
assert s.item() == s.iloc[0]

# using an ndarray like function
s = Series(np.random.randn(10))
result = Series(np.ones_like(s))
Expand Down