Skip to content

BUG: appending a Timedelta to Series incorrectly casts to integer #27303

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

Merged
merged 7 commits into from
Jul 10, 2019
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
1 change: 1 addition & 0 deletions doc/source/whatsnew/v0.25.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1051,6 +1051,7 @@ Indexing
- Bug in :class:`Categorical` and :class:`CategoricalIndex` with :class:`Interval` values when using the ``in`` operator (``__contains``) with objects that are not comparable to the values in the ``Interval`` (:issue:`23705`)
- Bug in :meth:`DataFrame.loc` and :meth:`DataFrame.iloc` on a :class:`DataFrame` with a single timezone-aware datetime64[ns] column incorrectly returning a scalar instead of a :class:`Series` (:issue:`27110`)
- Bug in :class:`CategoricalIndex` and :class:`Categorical` incorrectly raising ``ValueError`` instead of ``TypeError`` when a list is passed using the ``in`` operator (``__contains__``) (:issue:`21729`)
- Bug in setting a new value in a :class:`Series` with a :class:`Timedelta` object incorrectly casting the value to an integer (:issue:`22717`)
-

Missing
Expand Down
9 changes: 4 additions & 5 deletions pandas/core/indexing.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
is_sequence,
is_sparse,
)
from pandas.core.dtypes.concat import _concat_compat
from pandas.core.dtypes.generic import ABCDataFrame, ABCSeries
from pandas.core.dtypes.missing import _infer_fill_value, isna

Expand Down Expand Up @@ -429,11 +430,9 @@ def _setitem_with_indexer(self, indexer, value):
# this preserves dtype of the value
new_values = Series([value])._values
if len(self.obj._values):
try:
new_values = np.concatenate([self.obj._values, new_values])
except TypeError:
as_obj = self.obj.astype(object)
new_values = np.concatenate([as_obj, new_values])
# GH#22717 np.concatenate incorrect casts
# timedelta64 to integer
new_values = _concat_compat([self.obj._values, new_values])
self.obj._data = self.obj._constructor(
new_values, index=new_index, name=self.obj.name
)._data
Expand Down
25 changes: 25 additions & 0 deletions pandas/tests/series/indexing/test_indexing.py
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,31 @@ def test_timedelta_assignment():
tm.assert_series_equal(s, expected)
Copy link
Contributor

Choose a reason for hiding this comment

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

should really put this in a test_timedelta.py (and take the existing tests out of test_indexing).

Copy link
Member Author

Choose a reason for hiding this comment

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

after the current batch of PRs I'm planning on doing a review of the indexing tests. There are multiple dimensions along which we can sort/parametrize, any of which would be reasonable, but my guess is we are not being consistent about it.



@pytest.mark.parametrize(
"td",
[
pd.Timedelta("9 days"),
pd.Timedelta("9 days").to_timedelta64(),
pd.Timedelta("9 days").to_pytimedelta(),
],
)
def test_append_timedelta_does_not_cast(td):
# GH#22717 inserting a Timedelta should _not_ cast to int64
Copy link
Contributor

Choose a reason for hiding this comment

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

new test pls & parameterize over timedelta & np.timedelta64 as wel

expected = pd.Series(["x", td], index=[0, "td"], dtype=object)

# FIXME: these should either _both_ cast the object to Timedelta
# or both retain type(td)
ser = pd.Series(["x"])
ser["td"] = td
tm.assert_series_equal(ser, expected)
assert isinstance(ser["td"], pd.Timedelta)
Copy link
Member

Choose a reason for hiding this comment

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

Lost previous comment but can you use tm.assert_series_equal here? Also move to a separate test (test_timedelta_assignment_to_object?)

Copy link
Member Author

Choose a reason for hiding this comment

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

will do

Copy link
Contributor

Choose a reason for hiding this comment

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

can you compare vs the expected series instead


ser = pd.Series(["x"])
ser.loc["td"] = pd.Timedelta("9 days")
tm.assert_series_equal(ser, expected)
assert isinstance(ser["td"], pd.Timedelta)


def test_underlying_data_conversion():
# GH 4080
df = DataFrame({c: [1, 2, 3] for c in ["a", "b", "c"]})
Expand Down