BUG: Fix timedelta64+Timestamp, closes #24775 #26916

jbrockmendel · 2019-06-18T03:08:55Z

closes BUG: timedelta64 + Timestamp raises #24775
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

jreback · 2019-06-18T03:12:53Z

pandas/tests/scalar/timestamp/test_arithmetic.py

@@ -112,3 +112,9 @@ def test_addition_subtraction_preserve_frequency(self):
        td64 = np.timedelta64(1, 'D')
        assert (ts + td64).freq == original_freq
        assert (ts - td64).freq == original_freq
+
+    def test_radd_timedelta64(self):


seems like a lone test; can u co-locate with same for Timedelta and (or parametrize)

This is definitely the appropriate place. I think there are other tests in tests.scalar.timestamp that belong in this file

my point is to also include pd.Timedelta here as well via parameterization. I suspect we already test this elsewhere, so rather than add a one-off, should add elsewhere (or simply combine / move them here if this is the final destination).

(pandas) bash-3.2$ grep -r _radd pandas/tests/ --include '*.py' pandas/tests//tseries/offsets/test_offsets.py: def test_radd(self): pandas/tests//frame/test_query_eval.py: for op_str, op, rop in [('+', '__add__', '__radd__'), pandas/tests//arithmetic/test_datetime64.py: if op_str not in ['__add__', '__radd__', '__sub__']: pandas/tests//arithmetic/test_datetime64.py: if op_str not in ['__add__', '__radd__', '__sub__', '__rsub__']: pandas/tests//arithmetic/test_datetime64.py: @pytest.mark.parametrize('op', ['__add__', '__radd__', pandas/tests//arithmetic/test_timedelta64.py: # Tests for timedelta64[ns] __add__, __sub__, __radd__, __rsub__ pandas/tests//arithmetic/test_numeric.py: # __add__, __sub__, __radd__, __rsub__, __iadd__, __isub__ pandas/tests//arithmetic/test_numeric.py: def test_series_frame_radd_bug(self): pandas/tests//arithmetic/test_object.py: def test_objarr_radd_str(self, box): pandas/tests//arithmetic/test_object.py: def test_objarr_radd_str_invalid(self, dtype, data, box): pandas/tests//arithmetic/test_object.py: def test_series_with_dtype_radd_timedelta(self, dtype): pandas/tests//scalar/timedelta/test_arithmetic.py: __add__, __radd__, pandas/tests//scalar/timedelta/test_arithmetic.py: # datetime + Timedelta does _not_ call Timedelta.__radd__, pandas/tests//indexes/test_category.py: (lambda idx: ['a', 'b'] + idx, '__radd__'),

I suspect we already test this elsewhere, so rather than add a one-off, should add elsewhere (or simply combine / move them here if this is the final destination).

well we don't already test this specifically, since this test fails on master. Most of the grep results posted are for non-scalars; we definitely don't want to start mixing the scalar tests in with those. I'll collect other Timestamp arithmetic tests that are currently misplaced and put them in this file.

I suspect we already test this elsewhere, so rather than add a one-off, should add elsewhere (or simply combine / move them here if this is the final destination).

well we don't already test this specifically, since this test fails on master. Most of the grep results posted are for non-scalars; we definitely don't want to start mixing the scalar tests in with those. I'll collect other Timestamp arithmetic tests that are currently misplaced and put them in this file.

If you read my point is that we certaily test this for Timedelta. so these belong together.

doc/source/whatsnew/v0.25.0.rst

codecov · 2019-06-18T16:38:50Z

Codecov Report

Merging #26916 into master will decrease coverage by 50.76%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           master   #26916       +/-   ##
===========================================
- Coverage   91.87%    41.1%   -50.77%     
===========================================
  Files         180      180               
  Lines       50712    50712               
===========================================
- Hits        46590    20845    -25745     
- Misses       4122    29867    +25745

Flag	Coverage Δ
#multiple	`?`
#single	`41.1% <ø> (-0.08%)`	⬇️

Impacted Files	Coverage Δ
pandas/io/formats/latex.py	`0% <0%> (-100%)`	⬇️
pandas/plotting/_matplotlib/__init__.py	`0% <0%> (-100%)`	⬇️
pandas/io/gcs.py	`0% <0%> (-100%)`	⬇️
pandas/io/sas/sas_constants.py	`0% <0%> (-100%)`	⬇️
pandas/core/groupby/categorical.py	`0% <0%> (-100%)`	⬇️
pandas/tseries/plotting.py	`0% <0%> (-100%)`	⬇️
pandas/io/s3.py	`0% <0%> (-100%)`	⬇️
pandas/io/formats/html.py	`0% <0%> (-99.37%)`	⬇️
pandas/io/sas/sas7bdat.py	`0% <0%> (-91.16%)`	⬇️
pandas/io/sas/sas_xport.py	`0% <0%> (-90.1%)`	⬇️
... and 132 more

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 baa77c3...9666416. Read the comment docs.

codecov · 2019-06-18T16:38:58Z

Codecov Report

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

@@            Coverage Diff             @@
##           master   #26916      +/-   ##
==========================================
- Coverage   91.99%   91.98%   -0.01%     
==========================================
  Files         180      180              
  Lines       50774    50774              
==========================================
- Hits        46712    46707       -5     
- Misses       4062     4067       +5

Flag	Coverage Δ
#multiple	`90.63% <ø> (ø)`	⬆️
#single	`41.83% <ø> (-0.08%)`	⬇️

Impacted Files	Coverage Δ
pandas/io/gbq.py	`88.88% <0%> (-11.12%)`	⬇️
pandas/core/frame.py	`96.89% <0%> (-0.12%)`	⬇️
pandas/util/testing.py	`90.84% <0%> (-0.11%)`	⬇️

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 83fe8d7...534db4c. Read the comment docs.

jreback · 2019-06-19T00:07:40Z

pandas/tests/scalar/timestamp/test_arithmetic.py

@@ -112,3 +112,9 @@ def test_addition_subtraction_preserve_frequency(self):
        td64 = np.timedelta64(1, 'D')
        assert (ts + td64).freq == original_freq
        assert (ts - td64).freq == original_freq
+
+    def test_radd_timedelta64(self):


my point is to also include pd.Timedelta here as well via parameterization. I suspect we already test this elsewhere, so rather than add a one-off, should add elsewhere (or simply combine / move them here if this is the final destination).

(pandas) bash-3.2$ grep -r _radd pandas/tests/ --include '*.py' pandas/tests//tseries/offsets/test_offsets.py: def test_radd(self): pandas/tests//frame/test_query_eval.py: for op_str, op, rop in [('+', '__add__', '__radd__'), pandas/tests//arithmetic/test_datetime64.py: if op_str not in ['__add__', '__radd__', '__sub__']: pandas/tests//arithmetic/test_datetime64.py: if op_str not in ['__add__', '__radd__', '__sub__', '__rsub__']: pandas/tests//arithmetic/test_datetime64.py: @pytest.mark.parametrize('op', ['__add__', '__radd__', pandas/tests//arithmetic/test_timedelta64.py: # Tests for timedelta64[ns] __add__, __sub__, __radd__, __rsub__ pandas/tests//arithmetic/test_numeric.py: # __add__, __sub__, __radd__, __rsub__, __iadd__, __isub__ pandas/tests//arithmetic/test_numeric.py: def test_series_frame_radd_bug(self): pandas/tests//arithmetic/test_object.py: def test_objarr_radd_str(self, box): pandas/tests//arithmetic/test_object.py: def test_objarr_radd_str_invalid(self, dtype, data, box): pandas/tests//arithmetic/test_object.py: def test_series_with_dtype_radd_timedelta(self, dtype): pandas/tests//scalar/timedelta/test_arithmetic.py: __add__, __radd__, pandas/tests//scalar/timedelta/test_arithmetic.py: # datetime + Timedelta does _not_ call Timedelta.__radd__, pandas/tests//indexes/test_category.py: (lambda idx: ['a', 'b'] + idx, '__radd__'),

jreback · 2019-06-19T00:08:35Z

pandas/tests/scalar/timestamp/test_comparisons.py

+    def test_compare_zerodim_array(self):
+        # GH#26916
+        ts = Timestamp.now()
+        td64 = np.datetime64('2016-01-01', 'ns')


this is odd named td64 no?

typo, will update

jbrockmendel · 2019-06-19T01:16:03Z

Fixed typo, determined there's only a couple other dedicated Timestamp arith tests, moved them over to the appropriate place and parametrized them.

We've done a good job at making tslibs self-contained, less so with the tests. At some point I'd like to change that, hopefully make it easier to identify gaps in isolated coverage.

jreback · 2019-06-19T01:20:08Z

Fixed typo, determined there's only a couple other dedicated Timestamp arith tests, moved them over to the appropriate place and parametrized them.

We've done a good job at making tslibs self-contained, less so with the tests. At some point I'd like to change that, hopefully make it easier to identify gaps in isolated coverage.

yep i here ya increasing coverage always +1

jreback · 2019-06-21T01:25:45Z

pandas/tests/scalar/timestamp/test_arithmetic.py

+        td = np.timedelta64(3, 'h')
+        assert td + ts == ts + td
+
+    @pytest.mark.parametrize('other,exdiff', [


can you do: exdiff -> expected_difference

TomAugspurger

This seems good. Can you fix the merge conflict and merge on green?

jreback · 2019-06-24T16:12:29Z

i have some open requests here

jbrockmendel · 2019-06-24T16:41:58Z

i have some open requests here

I think your open request is that we put this with the Timedelta arithmetic tests. But doing so would break the organization of the Timedelta tests.

jreback · 2019-06-24T16:57:22Z

no my requests are to parametrize over a Timedelta the tests u added

jbrockmendel · 2019-06-24T17:13:48Z

no my requests are to parametrize over a Timedelta the tests u added

added.

jreback · 2019-06-25T12:48:18Z

thanks @jbrockmendel

BUG: Fix timedelta64+Timestamp, closes pandas-dev#24775

bf773e0

jreback requested changes Jun 18, 2019

View reviewed changes

jschendel reviewed Jun 18, 2019

View reviewed changes

doc/source/whatsnew/v0.25.0.rst Outdated Show resolved Hide resolved

jbrockmendel added 2 commits June 18, 2019 07:11

fix missing backticks

d5a4eef

fix comparison bug

9666416

jreback requested changes Jun 19, 2019

View reviewed changes

jreback added Timedelta Timedelta data type Datetime Datetime data dtype labels Jun 19, 2019

Collect Timestamp arith tests, parametrize

3c48d57

jreback added this to the 0.25.0 milestone Jun 21, 2019

jreback requested changes Jun 21, 2019

View reviewed changes

jbrockmendel added 2 commits June 20, 2019 18:48

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

eaa9b26

rename exdiff-->expected_difference

d486dcd

TomAugspurger approved these changes Jun 24, 2019

View reviewed changes

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

3268e70

jbrockmendel added 2 commits June 24, 2019 10:07

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

d10668a

parametrize test

534db4c

jreback approved these changes Jun 25, 2019

View reviewed changes

jreback merged commit f0919f2 into pandas-dev:master Jun 25, 2019

jbrockmendel deleted the tsadd branch June 25, 2019 14:10

pganssle mentioned this pull request Aug 4, 2020

REGR: Comparing timestamp to empty Series raises TypeError starting in 0.25.0 #35548

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: Fix timedelta64+Timestamp, closes #24775 #26916

BUG: Fix timedelta64+Timestamp, closes #24775 #26916

jbrockmendel commented Jun 18, 2019

jreback Jun 18, 2019

jbrockmendel Jun 18, 2019

jreback Jun 19, 2019

jbrockmendel Jun 19, 2019

jreback Jun 19, 2019

codecov bot commented Jun 18, 2019

codecov bot commented Jun 18, 2019 •

edited

Loading

jreback Jun 19, 2019

jreback Jun 19, 2019

jbrockmendel Jun 19, 2019

jbrockmendel commented Jun 19, 2019

jreback commented Jun 19, 2019

jreback Jun 21, 2019

TomAugspurger left a comment

jreback commented Jun 24, 2019

jbrockmendel commented Jun 24, 2019

jreback commented Jun 24, 2019

jbrockmendel commented Jun 24, 2019

jreback commented Jun 25, 2019

BUG: Fix timedelta64+Timestamp, closes #24775 #26916

BUG: Fix timedelta64+Timestamp, closes #24775 #26916

Conversation

jbrockmendel commented Jun 18, 2019

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

codecov bot commented Jun 18, 2019

Codecov Report

codecov bot commented Jun 18, 2019 • 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

jbrockmendel commented Jun 19, 2019

jreback commented Jun 19, 2019

Choose a reason for hiding this comment

TomAugspurger left a comment

Choose a reason for hiding this comment

jreback commented Jun 24, 2019

jbrockmendel commented Jun 24, 2019

jreback commented Jun 24, 2019

jbrockmendel commented Jun 24, 2019

jreback commented Jun 25, 2019

codecov bot commented Jun 18, 2019 •

edited

Loading