Fix #17965 datetime64 comparison #18188

watercrossing · 2017-11-09T11:15:07Z

closes COMPAT: datetime.date comparisons with datetime64[ns] #17965
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

TomAugspurger · 2017-11-09T11:55:46Z

pandas/tests/indexes/datetimes/test_partial_slicing.py

+    def test_comparison_to_datetime64(self):
+        # GH issue #17965
+        df = DataFrame({'dates': date_range('2000-01-01', '2000-01-05')})
+        date = df['dates'].unique()[0]


Is this the right test? date should be a datetime.date, and I think this is a datetime64[ns].

date is numpy.datetime64('2000-01-01T00:00:00.000000000'), which is consistent with the output of pd.date_range().values, which are datetime64[ns] too. Do you suggest this should be an array of datetime.date objects?

#17965 is about comparing with a single datetime.date object:

In [20]: pd.Timestamp('20130101').to_pydatetime().date()>=df.A ValueError: cannot set a Timestamp with a non-timestamp

paramaterize this test with date, datetime, np.datetime64, and Timestamp.
check for < and ==

True! I was not actually fixing the Issue, but rather this comment: #17965 (comment) - but it actually applies to both.

TomAugspurger · 2017-11-09T11:57:13Z

Python is inconsistent here:

python                                                                                                                                                                                                                               Thu Nov  9 05:53:21 2017
Python 3.6.1 (default, Apr  4 2017, 09:40:21)
[GCC 4.2.1 Compatible Apple LLVM 8.1.0 (clang-802.0.38)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import datetime
>>> datetime.datetime(2017, 1, 1) == datetime.date(2017, 1, 1)
False
>>> datetime.datetime(2017, 1, 1) >= datetime.date(2017, 1, 1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: can't compare datetime.datetime to datetime.date

Strange.... We should be a bit clearer on our policy if we're differing from Python. Are we saying that datetime.datetimes compare with a datetime.date by promoting the date to a datetime at midnight?

watercrossing · 2017-11-09T12:29:32Z

I don't think your comparison is correct - semantically the test is comparing two dates, not datetimes:
datetime.date(2013,11,4) == datetime.date(2013,11,4) is True - so we are not deviating from Python?

jreback · 2017-11-09T12:29:33Z

Strange.... We should be a bit clearer on our policy if we're differing from Python. Are we saying that datetime.datetimes compare with a datetime.date by promoting the date to a datetime at midnight?

python is inconcistent here, datetime.date should just compare to datetime.datetime, we just coerce everything to Timestamp in any event.

jreback · 2017-11-09T12:29:50Z

doc/source/whatsnew/v0.21.1.txt

@@ -61,6 +61,7 @@ Bug Fixes
 - Bug in :class:`DatetimeIndex` subtracting datetimelike from DatetimeIndex could fail to overflow (:issue:`18020`)
 - Bug in ``pd.Series.rolling.skew()`` and ``rolling.kurt()`` with all equal values has floating issue (:issue:`18044`)
 - Bug in ``pd.DataFrameGroupBy.count()`` when counting over a datetimelike column (:issue:`13393`)
+- Bug in ``_libs.index.convert_scalar()`` when comparing datetimes to np.datetime64 (:issue:`17965`)


make a user useful note

jreback · 2017-11-09T12:30:15Z

pandas/_libs/index.pyx

@@ -549,6 +549,8 @@ cpdef convert_scalar(ndarray arr, object value):
    if arr.descr.type_num == NPY_DATETIME:
        if isinstance(value, np.ndarray):
            pass
+        if isinstance(value, np.datetime64):
+            return Timestamp(value).value
        elif isinstance(value, datetime):


compbine np.datetime64 and datetime into the same clause

jreback · 2017-11-09T12:32:26Z

pandas/tests/indexes/datetimes/test_partial_slicing.py

+    def test_comparison_to_datetime64(self):
+        # GH issue #17965
+        df = DataFrame({'dates': date_range('2000-01-01', '2000-01-05')})
+        date = df['dates'].unique()[0]


paramaterize this test with date, datetime, np.datetime64, and Timestamp.
check for < and ==

jreback · 2017-11-09T12:32:54Z

pandas/tests/indexes/datetimes/test_partial_slicing.py

+        # GH issue #17965
+        df = DataFrame({'dates': date_range('2000-01-01', '2000-01-05')})
+        date = df['dates'].unique()[0]
+        tm.assert_frame_equal(df.loc[df['dates'] == date], df.iloc[:1])


use

result = expected = tm.assert_frame_equal(result, expected)

jreback · 2017-11-09T12:34:18Z

pandas/tests/indexes/datetimes/test_partial_slicing.py

@@ -330,3 +330,9 @@ def test_loc_datetime_length_one(self):

        result = df.loc['2016-10-01T00:00:00':]
        tm.assert_frame_equal(result, df)
+
+    def test_comparison_to_datetime64(self):
+        # GH issue #17965


use the frame from the original issue (which uses a NaT) as well.

jreback · 2017-11-09T12:45:02Z

@watercrossing the comparison is a non-converted datetime.date vs a Timestamp, which is what boxed versions of datetime64[ns] are. pls use the original issues exactly.

testing against a column of datetime.date is possible as well, though this is more tricky to construct.

jbrockmendel · 2017-11-09T15:40:28Z

Reiterating suggestion to coerce datetime.date to Period with freq=D

jreback · 2017-11-09T16:53:31Z

Reiterating suggestion to coerce datetime.date to Period with freq=D

not in this PR maybe at some point

jreback · 2017-11-10T13:46:45Z

doc/source/whatsnew/v0.21.1.txt

@@ -61,6 +61,7 @@ Bug Fixes
 - Bug in :class:`DatetimeIndex` subtracting datetimelike from DatetimeIndex could fail to overflow (:issue:`18020`)
 - Bug in ``pd.Series.rolling.skew()`` and ``rolling.kurt()`` with all equal values has floating issue (:issue:`18044`)
 - Bug in ``pd.DataFrameGroupBy.count()`` when counting over a datetimelike column (:issue:`13393`)
+- Bug in ``_libs.index.convert_scalar()``: Comparison between ``datetime64[ns]`` series and ``datetimelike`` (:issue:`17965`)


don't show internal functions here, rather the end-user effect, something like

a boolean comparison with a datetime.date and a datetime64[ns] dtype Series

jreback · 2017-11-10T13:48:17Z

pandas/tests/indexes/datetimes/test_partial_slicing.py

+    comparisonTS = Timestamp('20130101')
+
+    @pytest.mark.parametrize('datetimelike,opExp', product(
+        [comparisonTS, comparisonTS.to_pydatetime(),


directly construct these, e.g. Timestamp('20130101'), datetime.datetime(2013, 1, 1) etc

jreback · 2017-11-10T13:48:44Z

pandas/tests/indexes/datetimes/test_partial_slicing.py

+    @pytest.mark.parametrize('datetimelike,opExp', product(
+        [comparisonTS, comparisonTS.to_pydatetime(),
+         comparisonTS.to_pydatetime().date(), comparisonTS.to_datetime64()],
+        [(op.lt, [True, False, False, False]),


don't pass opExp, rather pass op (e.g. the op.lt) and the expected (the boolean result)

Can do, but i am not sure something like:

@pytest.mark.parametrize('datetimelike,op,expected', [(x, y[0], y[1]) for x, y in product( [pd.Timestamp('20130101'), datetime.datetime(2013, 1, 1), datetime.date(2013, 1, 1), np.datetime64('2013-01-01', 'ns')], [(op.lt, [True, False, False, False]), (op.le, [True, True, False, False]), (op.eq, [False, True, False, False]), (op.gt, [False, False, False, True])])])

is more readable.

sorry didn't see the product. instead use multiple paramatrize to achieve product.

eg..

@pytest.mark.parametrize(.....) @pytest.mark.parametrize('datetimelike'

jreback · 2017-11-10T14:04:27Z

rebase as well

codecov · 2017-11-13T11:15:05Z

Codecov Report

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

@@            Coverage Diff             @@
##           master   #18188      +/-   ##
==========================================
- Coverage   91.42%   91.38%   -0.05%     
==========================================
  Files         163      163              
  Lines       50068    50068              
==========================================
- Hits        45777    45756      -21     
- Misses       4291     4312      +21

Flag	Coverage Δ
#multiple	`89.19% <ø> (-0.03%)`	⬇️
#single	`40.35% <ø> (-0.06%)`	⬇️

Impacted Files	Coverage Δ
pandas/io/gbq.py	`25% <0%> (-58.34%)`	⬇️
pandas/plotting/_converter.py	`63.38% <0%> (-1.82%)`	⬇️
pandas/core/frame.py	`97.8% <0%> (-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 9050e38...a56f7fe. Read the comment docs.

codecov · 2017-11-13T11:15:11Z

Codecov Report

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

@@            Coverage Diff             @@
##           master   #18188      +/-   ##
==========================================
+ Coverage   91.38%   91.38%   +<.01%     
==========================================
  Files         164      164              
  Lines       49884    49884              
==========================================
+ Hits        45584    45586       +2     
+ Misses       4300     4298       -2

Flag	Coverage Δ
#multiple	`89.19% <ø> (+0.02%)`	⬆️
#single	`39.42% <ø> (-0.07%)`	⬇️

Impacted Files	Coverage Δ
pandas/io/gbq.py	`25% <0%> (-58.34%)`	⬇️
pandas/core/frame.py	`97.8% <0%> (-0.1%)`	⬇️
pandas/core/indexes/datetimes.py	`95.38% <0%> (-0.1%)`	⬇️
pandas/plotting/_converter.py	`65.25% <0%> (+1.81%)`	⬆️

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 7f4c960...d9f12f4. Read the comment docs.

jreback

small doc-change. ping on green.

jreback · 2017-11-13T12:56:11Z

doc/source/whatsnew/v0.21.1.txt

@@ -62,6 +62,7 @@ Bug Fixes
 - Bug in ``pd.Series.rolling.skew()`` and ``rolling.kurt()`` with all equal values has floating issue (:issue:`18044`)
 - Bug in ``pd.DataFrameGroupBy.count()`` when counting over a datetimelike column (:issue:`13393`)
 - Bug in ``pd.concat`` when empty and non-empty DataFrames or Series are concatenated (:issue:`18178` :issue:`18187`)
+- Bug in a boolean comparison with a ``datetime.datetime``, ``datetime.date``, ``np.datetime64`` and a ``datetime64[ns]`` dtype Series (:issue:`17965`)


just list datetime.datetime (as that's the actual bug)

move to the indexing section

@jreback Done.

jreback · 2017-11-13T13:12:02Z

ok ping on green.

watercrossing · 2017-11-13T13:15:49Z

@jreback: I have changed the whatsnew and rebased.

TomAugspurger · 2017-11-13T13:16:34Z

@watercrossing let us know if / when you see all the CI workers finish and we'll merge.

jreback · 2017-11-13T13:26:45Z

@watercrossing let us know if / when you see all the CI workers finish and we'll merge.

@TomAugspurger optimist!

watercrossing · 2017-11-14T12:04:45Z

@TomAugspurger / @jreback: Ping!

jreback · 2017-11-14T13:04:39Z

thanks @watercrossing

…pandas-dev#18188) (cherry picked from commit 77f10f0)

(cherry picked from commit 77f10f0)

TomAugspurger reviewed Nov 9, 2017

View reviewed changes

TomAugspurger added Dtype Conversions Unexpected or buggy dtype conversions Datetime Datetime data dtype labels Nov 9, 2017

jreback requested changes Nov 9, 2017

View reviewed changes

watercrossing force-pushed the timestampComparison branch 2 times, most recently from 8b01fe3 to ddfe7dc Compare November 10, 2017 11:06

jreback requested changes Nov 10, 2017

View reviewed changes

watercrossing force-pushed the timestampComparison branch from ddfe7dc to ebe805b Compare November 10, 2017 16:32

jreback approved these changes Nov 13, 2017

View reviewed changes

jreback added this to the 0.21.1 milestone Nov 13, 2017

Fix pandas-dev#17965 to allow full comparison of datetimelike objects

d9f12f4

watercrossing force-pushed the timestampComparison branch from 8b98651 to d9f12f4 Compare November 13, 2017 13:14

jreback added the Needs Backport label Nov 14, 2017

jreback merged commit 77f10f0 into pandas-dev:master Nov 14, 2017

zippeurfou mentioned this pull request Nov 29, 2017

datetime comparison from pandas.core.seroes.Series to numpy.datetime64 fail for pandas 0.21 #18569

Closed

jreback mentioned this pull request Dec 6, 2017

Regression: cannot set a Timestamp with a non-timestamp #18660

Closed

TomAugspurger pushed a commit to TomAugspurger/pandas that referenced this pull request Dec 8, 2017

Fix pandas-dev#17965 to allow full comparison of datetimelike objects (…

ea94667

…pandas-dev#18188) (cherry picked from commit 77f10f0)

TomAugspurger pushed a commit that referenced this pull request Dec 11, 2017

Fix #17965 to allow full comparison of datetimelike objects (#18188)

ee68bb3

(cherry picked from commit 77f10f0)

TomAugspurger removed the Needs Backport label Dec 11, 2017

TomAugspurger mentioned this pull request Jan 18, 2018

Fix DTI comparison with None, datetime.date #19301

Merged

4 tasks

TomAugspurger mentioned this pull request May 21, 2018

datetime.date no longer coerced to datetime64 for comparison operations #21152

Closed

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #17965 datetime64 comparison #18188

Fix #17965 datetime64 comparison #18188

watercrossing commented Nov 9, 2017

TomAugspurger Nov 9, 2017

watercrossing Nov 9, 2017

TomAugspurger Nov 9, 2017

jreback Nov 9, 2017

watercrossing Nov 9, 2017

TomAugspurger commented Nov 9, 2017

watercrossing commented Nov 9, 2017

jreback commented Nov 9, 2017

jreback Nov 9, 2017

jreback Nov 9, 2017

jreback Nov 9, 2017

jreback Nov 9, 2017

jreback Nov 9, 2017

jreback commented Nov 9, 2017

jbrockmendel commented Nov 9, 2017

jreback commented Nov 9, 2017

jreback Nov 10, 2017

jreback Nov 10, 2017

jreback Nov 10, 2017

watercrossing Nov 10, 2017 •

edited

Loading

jreback Nov 10, 2017

jreback commented Nov 10, 2017

codecov bot commented Nov 13, 2017

codecov bot commented Nov 13, 2017 •

edited

Loading

jreback left a comment

jreback Nov 13, 2017

jreback Nov 13, 2017

watercrossing Nov 13, 2017

jreback commented Nov 13, 2017

watercrossing commented Nov 13, 2017

TomAugspurger commented Nov 13, 2017

jreback commented Nov 13, 2017

watercrossing commented Nov 14, 2017

jreback commented Nov 14, 2017

Fix #17965 datetime64 comparison #18188

Fix #17965 datetime64 comparison #18188

Conversation

watercrossing commented Nov 9, 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

TomAugspurger commented Nov 9, 2017

watercrossing commented Nov 9, 2017

jreback commented Nov 9, 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

jreback commented Nov 9, 2017

jbrockmendel commented Nov 9, 2017

jreback commented Nov 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

watercrossing Nov 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Nov 10, 2017

codecov bot commented Nov 13, 2017

Codecov Report

codecov bot commented Nov 13, 2017 • edited Loading

Codecov Report

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

jreback commented Nov 13, 2017

watercrossing commented Nov 13, 2017

TomAugspurger commented Nov 13, 2017

jreback commented Nov 13, 2017

watercrossing commented Nov 14, 2017

jreback commented Nov 14, 2017

watercrossing Nov 10, 2017 •

edited

Loading

codecov bot commented Nov 13, 2017 •

edited

Loading