BUG: fix Series[timedelta64] arithmetic with Timedelta scalars #18831

jbrockmendel · 2017-12-19T02:21:05Z

Before:

>>> ser = pd.Series([pd.Timedelta(minutes=5, seconds=3)] * 3)
>>> ser.iloc[-1] = np.nan
>>> scalar = pd.Timedelta(minutes=5, seconds=4)
>>> ser // scalar
[...]
TypeError: can only operate on a timedeltas for addition, subtraction, and division, but the operator [__floordiv__] was passed

>>> scalar / ser
0   00:00:00.000000
1   00:00:00.000000
2               NaT
dtype: timedelta64[ns]

>>> scalar.to_pytimedelta() / ser
0   00:00:00.000000
1   00:00:00.000000
2               NaT
dtype: timedelta64[ns]

>>> scalar.to_timedelta64() / ser
0   00:00:00.000000
1   00:00:00.000000
2               NaT
dtype: timedelta64[ns]

>>> scalar.to_pytimedelta() // ser
[...]
TypeError: can only operate on a timedeltas for addition, subtraction, and division, but the operator [__rfloordiv__] was passed

>>> scalar.to_timedelta64() // ser
[...]
TypeError: can only operate on a timedeltas for addition, subtraction, and division, but the operator [__rfloordiv__] was passed

After

>>> ser // scalar
0    0.0
1    0.0
2    NaN
dtype: float64

>>> scalar / ser
0    1.0033
1    1.0033
2       NaN
dtype: float64

>>> scalar.to_pytimedelta() / ser
0    1.0033
1    1.0033
2       NaN
dtype: float64

>>> scalar.to_timedelta64() / ser
0    1.0033
1    1.0033
2       NaN
dtype: float64

>>> scalar.to_pytimedelta() // ser
0    1.0
1    1.0
2    NaN
dtype: float64

>>> scalar.to_timedelta64() // ser
0    1.0
1    1.0
2    NaN
dtype: float64

Also identified a bug in Timedelta.__floordiv__, noted it in #18824, will address that separately.

closes #xxxx
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

jbrockmendel · 2017-12-19T04:20:50Z

Travis timeout

codecov · 2017-12-19T05:38:34Z

Codecov Report

Merging #18831 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #18831      +/-   ##
==========================================
- Coverage   91.58%   91.58%   -0.01%     
==========================================
  Files         150      150              
  Lines       48972    48972              
==========================================
- Hits        44851    44849       -2     
- Misses       4121     4123       +2

Flag	Coverage Δ
#multiple	`89.94% <ø> (-0.01%)`	⬇️
#single	`41.72% <ø> (ø)`	⬆️

Impacted Files	Coverage Δ
pandas/core/ops.py	`90.24% <ø> (ø)`	⬆️
pandas/util/testing.py	`84.74% <0%> (-0.22%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 21f2412...729d240. Read the comment docs.

…medelta_scalars

gfyoung · 2017-12-19T23:09:22Z

pandas/tests/series/test_operators.py

+                               Series([0, 0, np.nan]))
+        # TODO: the Timedelta // td1 fails because of a bug
+        # in Timedelta.__floordiv__, see GH#18824
+        # tm.assert_series_equal(tdscalar // td1, Series([1, 1, np.nan]))


I would consider breaking out into a separate test so we can xfail it separately.

Works for me. I've got a fix ready pending feedback on #18846.

yes would clean up these tests here (IIRC you broke these out by operator elsewhere)

jreback · 2017-12-20T13:32:17Z

doc/source/whatsnew/v0.22.0.txt

@@ -354,3 +354,4 @@ Other

 - Improved error message when attempting to use a Python keyword as an identifier in a ``numexpr`` backed query (:issue:`18221`)
 - Bug in :class:`Timestamp` where comparison with an array of ``Timestamp`` objects would result in a ``RecursionError`` (:issue:`15183`)
+- Bug in :func:`Series.__floordiv__`  and :func:`Series.__rfloordiv__` where operating on a scalar ``timedelta`` raises an exception (:issue:`18824`)


don't put things in Other. This is conversion. Also note that a floordiv of scalar / Series was incorrect before.

verify that we have a doc example in timedelta.rst as well.

Also note that a floordiv of scalar / Series was incorrect before.

Something other than the mention of Series.__rfloordiv__?

jreback · 2017-12-20T13:33:13Z

pandas/tests/series/test_operators.py

+                               Series([0, 0, np.nan]))
+        # TODO: the Timedelta // td1 fails because of a bug
+        # in Timedelta.__floordiv__, see GH#18824
+        # tm.assert_series_equal(tdscalar // td1, Series([1, 1, np.nan]))


yes would clean up these tests here (IIRC you broke these out by operator elsewhere)

…section

jreback · 2017-12-21T15:08:47Z

pandas/tests/series/test_operators.py

+        tdscalar - td1
+        td1 / tdscalar
+        tdscalar / td1
+        tm.assert_series_equal(td1 // tdscalar, Series([0, 0, np.nan]))


can you parametrize these last ones they seem pretty similar

This is part of a 127 line test that is all over the place. I'd prefer to clean it up in a follow-up.

ok, this needs parameterization / refactor, but can follow up.

jreback · 2017-12-21T15:15:51Z

pandas/tests/series/test_operators.py

@@ -962,6 +962,14 @@ def test_timedelta64_ops_nat(self):


 class TestDatetimeSeriesArithmetic(object):
+    @pytest.mark.xfail(reason='GH#18824 bug in Timedelta.__floordiv__')


is this the right issue?

AFAIK there is no dedicated issue. I made a checkbox for this in 18224.

don't mark 18224, its way to big to figure out what you are talking about (you can certainly reference this PR which is I think what you are doing). rather list the PR number (if there isn't an issue). do this generally.

…medelta_scalars

jbrockmendel · 2017-12-23T02:18:29Z

I think this, #18877, #18875, #18832, and #18783 have all made it through the gauntlet. #18884 and #18762 need clarification on the requested reivisions. Everything else needs attention and will hopefully get it over the weekend.

jreback

lgtm. some doc changes. ping on green.

jreback · 2017-12-23T20:06:44Z

pandas/tests/series/test_operators.py

@@ -962,6 +962,14 @@ def test_timedelta64_ops_nat(self):


 class TestDatetimeSeriesArithmetic(object):
+    @pytest.mark.xfail(reason='GH#18824 bug in Timedelta.__floordiv__')


don't mark 18224, its way to big to figure out what you are talking about (you can certainly reference this PR which is I think what you are doing). rather list the PR number (if there isn't an issue). do this generally.

jreback · 2017-12-23T20:07:02Z

doc/source/whatsnew/v0.23.0.txt

@@ -282,6 +282,7 @@ Conversion
 - Bug in :meth:`Index.astype` with a categorical dtype where the resultant index is not converted to a :class:`CategoricalIndex` for all types of index (:issue:`18630`)
 - Bug in :meth:`Series.astype` and ``Categorical.astype()`` where an existing categorical data does not get updated (:issue:`10696`, :issue:`18593`)
 - Bug in :class:`Series` constructor with an int or float list where specifying ``dtype=str``, ``dtype='str'`` or ``dtype='U'`` failed to convert the data elements to strings (:issue:`16605`)
+- Bug in :func:`Series.__floordiv__`  and :func:`Series.__rfloordiv__` where operating on a scalar ``timedelta`` raises an exception (:issue:`18824`)


I would just say floor division here.

jreback · 2017-12-23T20:07:46Z

pandas/tests/series/test_operators.py

+        tdscalar - td1
+        td1 / tdscalar
+        tdscalar / td1
+        tm.assert_series_equal(td1 // tdscalar, Series([0, 0, np.nan]))


ok, this needs parameterization / refactor, but can follow up.

…medelta_scalars

jbrockmendel · 2017-12-24T17:32:43Z

Ping

…medelta_scalars

jbrockmendel · 2017-12-28T17:11:28Z

Looks like a test got lost in the last merge, will need to track that down. If there's a way to cancel the CI run, go for it.

jbrockmendel · 2017-12-28T22:06:08Z

Merge/rebase issue has been resolved. This should be good to go (along with #18849 and #18883).

jreback · 2017-12-29T00:20:57Z

pandas/tests/series/test_operators.py

+    @pytest.mark.parametrize('scalar_td', [
+        timedelta(minutes=5, seconds=4),
+        pytest.param(Timedelta('5m4s'),
+                     marks=pytest.mark.xfail(reason="Timedelta.__floordiv__ "


floordiv -> rfloordiv

? Both Timedelta.__floordiv__ and Timedelta.__rfloordiv__ are wonky. Or are you referring to something else?

you reason is wrong, it should be rfloordiv

You mean the reason="Timedelta.__floordiv__...? The test fails because Timedelta.__floordiv__ gets called, not Timedelta.__rfloordiv__.

jreback · 2017-12-29T00:22:04Z

pandas/tests/series/test_operators.py

+        expected = Series([0, 0, np.nan])
+        tm.assert_series_equal(result, expected)
+
+        # We can test __rfloordiv__ using this syntax,


shouldn't this be in the other test? (as the other one is rfloordiv), but this seems to work?

It's kind of a man without a country regardless. I didn't want to put it in the rfloordiv test because that was xfailed so putting it there would be akin to not running it at all. But you're right it doesn't really belong in the floordiv test either. I'm open to suggestions.

then make another test for it

…medelta_scalars

jreback · 2017-12-31T17:26:14Z

thanks!

…s-dev#18831)

jbrockmendel added 3 commits December 18, 2017 17:53

Fix Series timedelta64 // timedelta

1eab96f

allow rtruediv

25071a8

amend test case with pytimedelta, timedelta64

45c7260

jbrockmendel changed the title ~~Timedelta scalars~~ BUG: fix Series[timedelta64] arithmetic with Timedelta scalars Dec 19, 2017

This was referenced Dec 19, 2017

Refactor _TimeOp._validate to separate datetime vs timedelta vs dateoffset #18832

Merged

DataFrame vs Series vs Index arithmetic Roundup #18824

Closed

dummy commit to force CI

95cc5f9

Merge branch 'master' of https://github.com/pandas-dev/pandas into ti…

6ff2d5f

…medelta_scalars

gfyoung added Timedelta Timedelta data type Datetime Datetime data dtype labels Dec 19, 2017

gfyoung reviewed Dec 19, 2017

View reviewed changes

gfyoung added the Bug label Dec 19, 2017

jreback requested changes Dec 20, 2017

View reviewed changes

jbrockmendel added 2 commits December 20, 2017 10:28

xfail test

4fe8f74

floordiv example in timedeltas.rst, move whatsnew note to conversion …

4821f05

…section

jreback requested changes Dec 21, 2017

View reviewed changes

jbrockmendel added 2 commits December 21, 2017 08:58

Merge branch 'master' of https://github.com/pandas-dev/pandas into ti…

10054de

…medelta_scalars

Merge branch 'master' of https://github.com/pandas-dev/pandas into ti…

66561ef

…medelta_scalars

jreback requested changes Dec 23, 2017

View reviewed changes

jreback added this to the 0.23.0 milestone Dec 23, 2017

jbrockmendel added 2 commits December 23, 2017 16:16

Merge branch 'master' of https://github.com/pandas-dev/pandas into ti…

347a221

…medelta_scalars

edit issue reference

8789be7

jbrockmendel added 2 commits December 27, 2017 13:19

Merge branch 'master' of https://github.com/pandas-dev/pandas into ti…

c3795d0

…medelta_scalars

Merge branch 'master' of https://github.com/pandas-dev/pandas into ti…

ef8d6e2

…medelta_scalars

re-implement tests lost in rebase/merge mixups

2b3484f

jreback requested changes Dec 29, 2017

View reviewed changes

jbrockmendel added 3 commits December 28, 2017 20:29

Merge branch 'master' of https://github.com/pandas-dev/pandas into ti…

3250913

…medelta_scalars

Merge branch 'master' of https://github.com/pandas-dev/pandas into ti…

3a5c3b5

…medelta_scalars

separate explicit rfloordiv test

729d240

jreback approved these changes Dec 29, 2017

View reviewed changes

jbrockmendel mentioned this pull request Dec 31, 2017

Make Series[datetime64] - pd.NaT behave like DatetimeIndex - pd.NaT #18960

Closed

4 tasks

jreback merged commit cfa5ea6 into pandas-dev:master Dec 31, 2017

hexgnu pushed a commit to hexgnu/pandas that referenced this pull request Jan 1, 2018

BUG: fix Series[timedelta64] arithmetic with Timedelta scalars (panda…

a864a0f

…s-dev#18831)

jbrockmendel deleted the timedelta_scalars branch February 11, 2018 22:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: fix Series[timedelta64] arithmetic with Timedelta scalars #18831

BUG: fix Series[timedelta64] arithmetic with Timedelta scalars #18831

jbrockmendel commented Dec 19, 2017 •

edited by jreback

Loading

jbrockmendel commented Dec 19, 2017

codecov bot commented Dec 19, 2017 •

edited

Loading

gfyoung Dec 19, 2017

jbrockmendel Dec 19, 2017

jreback Dec 20, 2017

jreback Dec 20, 2017

jreback Dec 20, 2017

jbrockmendel Dec 20, 2017

jreback Dec 20, 2017

jreback Dec 21, 2017

jbrockmendel Dec 21, 2017

jreback Dec 23, 2017

jreback Dec 21, 2017

jbrockmendel Dec 21, 2017

jreback Dec 23, 2017

jbrockmendel commented Dec 23, 2017

jreback left a comment

jreback Dec 23, 2017

jreback Dec 23, 2017

jreback Dec 23, 2017

jbrockmendel commented Dec 24, 2017

jbrockmendel commented Dec 28, 2017

jbrockmendel commented Dec 28, 2017

jreback Dec 29, 2017

jbrockmendel Dec 29, 2017

jreback Dec 29, 2017

jbrockmendel Dec 29, 2017

jreback Dec 29, 2017

jbrockmendel Dec 29, 2017

jreback Dec 29, 2017

jreback commented Dec 31, 2017

		@@ -962,6 +962,14 @@ def test_timedelta64_ops_nat(self):


		class TestDatetimeSeriesArithmetic(object):
		@pytest.mark.xfail(reason='GH#18824 bug in Timedelta.__floordiv__')

BUG: fix Series[timedelta64] arithmetic with Timedelta scalars #18831

BUG: fix Series[timedelta64] arithmetic with Timedelta scalars #18831

Conversation

jbrockmendel commented Dec 19, 2017 • edited by jreback Loading

jbrockmendel commented Dec 19, 2017

codecov bot commented Dec 19, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbrockmendel commented Dec 23, 2017

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbrockmendel commented Dec 24, 2017

jbrockmendel commented Dec 28, 2017

jbrockmendel commented Dec 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Dec 31, 2017

jbrockmendel commented Dec 19, 2017 •

edited by jreback

Loading

codecov bot commented Dec 19, 2017 •

edited

Loading