Fix arithmetic errors with timedelta64 dtypes #22390

jbrockmendel · 2018-08-16T20:35:04Z

Kind of surprising, but I don't see any Issues for these bugs. Might be because many of them were found after parametrizing arithmetic tests, so instead of Issues we had xfails.

xref #20088
xref #22163

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

codecov · 2018-08-16T21:32:46Z

Codecov Report

Merging #22390 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #22390      +/-   ##
==========================================
- Coverage   92.04%   92.04%   -0.01%     
==========================================
  Files         169      169              
  Lines       50740    50759      +19     
==========================================
+ Hits        46705    46722      +17     
- Misses       4035     4037       +2

Flag	Coverage Δ
#multiple	`90.45% <100%> (-0.01%)`	⬇️
#single	`42.24% <36.84%> (-0.01%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/indexes/base.py	`96.45% <100%> (+0.01%)`	⬆️
pandas/core/indexes/range.py	`95.73% <100%> (+0.02%)`	⬆️
pandas/core/ops.py	`96.76% <100%> (+0.04%)`	⬆️
pandas/core/arrays/timedeltas.py	`91.54% <0%> (-1%)`	⬇️

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 25e6a21...cdfb3bf. Read the comment docs.

jreback · 2018-08-17T10:05:20Z

doc/source/whatsnew/v0.24.0.txt

@@ -576,7 +576,10 @@ Datetimelike
 - Bug in :class:`DataFrame` with mixed dtypes including ``datetime64[ns]`` incorrectly raising ``TypeError`` on equality comparisons (:issue:`13128`,:issue:`22163`)
 - Bug in :meth:`DataFrame.eq` comparison against ``NaT`` incorrectly returning ``True`` or ``NaN`` (:issue:`15697`,:issue:`22163`)
 - Bug in :class:`DataFrame` with ``timedelta64[ns]`` dtype division by ``Timedelta``-like scalar incorrectly returning ``timedelta64[ns]`` dtype instead of ``float64`` dtype (:issue:`20088`,:issue:`22163`)


can you split out to a timedelta bug section (as this is getting kind of long)?

can be in a future PR

Sounds good

I think we now have a separate section?

jreback · 2018-08-17T10:07:16Z

pandas/core/indexes/range.py

@@ -596,6 +597,9 @@ def _evaluate_numeric_binop(self, other):
                    # GH#19333 is_integer evaluated True on timedelta64,
                    # so we need to catch these explicitly
                    return op(self._int64index, other)
+                elif is_timedelta64_dtype(other):


doesn't the fall thru raise TypeError?

I don’t understand the question

your comment is at odds with the checking i think. pls revise comments and/or checks.

The comment below this line # must be an np.ndarray; GH#22390 along with the check above elif is_timedelta64_dtype(other) is just saying this is an ndarray["timedelta64['ns']"]. I don't think these are at odds.

jreback · 2018-08-17T10:09:49Z

pandas/core/ops.py

+            # timedelta64 dtypes because numpy casts integer dtypes to
+            # timedelta64 when operating with timedelta64
+            if isinstance(right, np.ndarray):
+                # upcast to TimedeltaIndex before dispatching


uhh this seems like a lot of special casing, any way to make this simpler

The only simplification I considered but didn't use was something like:

def maybe_upcast_for_op(obj): if type(obj) is timedelta: return pd.Timedelta(obj) if isinstance(obj, np.ndarray) and is_timedelta64_dtype(obj): return pd.TimedeltaIndex(obj) return obj

which would go right after get_op_result_name.

maybe re-route the scalars to a separate path. This is getting way special casey here.

I'll implement the maybe_upcast idea and see if that is prettier

jbrockmendel · 2018-08-17T14:26:41Z

This will probably need rebasing after #22350 goes through

jreback · 2018-08-20T10:45:30Z

pandas/core/ops.py

+                                    index=left.index, name=res_name,
+                                    dtype=result.dtype)
+
+        elif type(right) is datetime.timedelta:


this case you can include above (before what you added), no?

if isintance(right, (datetime.timedelta, Timedelta)): right = Timedelta(right) ....

The version code above casts to TimedeltaIndex, not Timedelta.

jreback · 2018-08-20T10:46:04Z

pandas/core/ops.py

+            # timedelta64 dtypes because numpy casts integer dtypes to
+            # timedelta64 when operating with timedelta64
+            if isinstance(right, np.ndarray):
+                # upcast to TimedeltaIndex before dispatching


maybe re-route the scalars to a separate path. This is getting way special casey here.

jreback · 2018-08-20T10:47:17Z

pandas/core/indexes/range.py

@@ -596,6 +597,9 @@ def _evaluate_numeric_binop(self, other):
                    # GH#19333 is_integer evaluated True on timedelta64,
                    # so we need to catch these explicitly
                    return op(self._int64index, other)
+                elif is_timedelta64_dtype(other):


your comment is at odds with the checking i think. pls revise comments and/or checks.

jbrockmendel · 2018-08-20T16:22:48Z

Implemented maybe_upcast_for_op. There may be other places where this can be used; I'll take a look in a follow-up.

jreback

minor doc-comment. rebase and ping on green.

jreback · 2018-08-22T12:34:53Z

doc/source/whatsnew/v0.24.0.txt

@@ -576,7 +576,10 @@ Datetimelike
 - Bug in :class:`DataFrame` with mixed dtypes including ``datetime64[ns]`` incorrectly raising ``TypeError`` on equality comparisons (:issue:`13128`,:issue:`22163`)
 - Bug in :meth:`DataFrame.eq` comparison against ``NaT`` incorrectly returning ``True`` or ``NaN`` (:issue:`15697`,:issue:`22163`)
 - Bug in :class:`DataFrame` with ``timedelta64[ns]`` dtype division by ``Timedelta``-like scalar incorrectly returning ``timedelta64[ns]`` dtype instead of ``float64`` dtype (:issue:`20088`,:issue:`22163`)


I think we now have a separate section?

jbrockmendel · 2018-08-23T00:46:51Z

ping

jreback · 2018-08-23T10:37:12Z

thanks! nice cleaning

jbrockmendel added 9 commits August 16, 2018 09:53

un-xfail tests fixed by pandas-dev#22163

05b2c50

Fix Index[object] op with Series[timedelta64]

6dbb2e6

Fix multiplication of timedelta with Series

8c3df15

un-xfail no-longer-broken tests

a7502ea

Fix numeric Series add/sub timedelta64array/index

ef14164

Fix/test numeric index ops with timedelta64 array/index/series

6b02691

flake8 fixup

b856744

add GH references

c493b23

Fixup remove unused import

000e6df

jreback requested changes Aug 17, 2018

View reviewed changes

jreback added Bug Timedelta Timedelta data type labels Aug 17, 2018

jreback requested changes Aug 20, 2018

View reviewed changes

jbrockmendel added 2 commits August 20, 2018 09:12

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

19a18ee

implement maybe_upcast_for_op

6bd5d0e

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

16e0cfe

jreback approved these changes Aug 22, 2018

View reviewed changes

jreback added this to the 0.24.0 milestone Aug 22, 2018

jbrockmendel added 2 commits August 22, 2018 06:51

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

40463a6

move notes to Timedelta section

cdfb3bf

jreback merged commit dd24e76 into pandas-dev:master Aug 23, 2018

jbrockmendel deleted the rbug branch August 23, 2018 15:46

Sup3rGeo pushed a commit to Sup3rGeo/pandas that referenced this pull request Oct 1, 2018

Fix arithmetic errors with timedelta64 dtypes (pandas-dev#22390)

77f0b15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix arithmetic errors with timedelta64 dtypes #22390

Fix arithmetic errors with timedelta64 dtypes #22390

jbrockmendel commented Aug 16, 2018 •

edited by jreback

Loading

codecov bot commented Aug 16, 2018 •

edited

Loading

jreback Aug 17, 2018

jbrockmendel Aug 17, 2018

jreback Aug 22, 2018

jreback Aug 17, 2018

jbrockmendel Aug 18, 2018

jreback Aug 20, 2018

jbrockmendel Aug 20, 2018

jreback Aug 17, 2018

jbrockmendel Aug 17, 2018

jreback Aug 20, 2018

jbrockmendel Aug 20, 2018

jbrockmendel commented Aug 17, 2018

jreback Aug 20, 2018

jbrockmendel Aug 20, 2018

jreback Aug 20, 2018

jreback Aug 20, 2018

jbrockmendel commented Aug 20, 2018

jreback left a comment

jreback Aug 22, 2018

jbrockmendel commented Aug 23, 2018

jreback commented Aug 23, 2018

Fix arithmetic errors with timedelta64 dtypes #22390

Fix arithmetic errors with timedelta64 dtypes #22390

Conversation

jbrockmendel commented Aug 16, 2018 • edited by jreback Loading

codecov bot commented Aug 16, 2018 • 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

jbrockmendel commented Aug 17, 2018

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 Aug 20, 2018

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbrockmendel commented Aug 23, 2018

jreback commented Aug 23, 2018

jbrockmendel commented Aug 16, 2018 •

edited by jreback

Loading

codecov bot commented Aug 16, 2018 •

edited

Loading