[Bug] Fix various DatetimeIndex comparison bugs #22074

jbrockmendel · 2018-07-27T03:17:30Z

Will have to see which issues this closes. This is also a precursor to upcoming PR that dispatches DataFrame ops to Series (which in turn dispatches to Index).

Fixes #7830 (the OP's example is already fixed, this fixes examples in the comments to that issue)
Fixes DTI half of #19804

closes Equality comparison raises exception #7830
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

jbrockmendel · 2018-07-27T03:20:29Z

pandas/core/arrays/datetimes.py

-                raise TypeError('%s type object %s' %
-                                (type(other), str(other)))
-
-            if is_datetimelike(other):


This previously called self._assert_tzawareness_compat(other) for other with timedelta64-dtype

(Really, is_datetimelike is a footgun)

jbrockmendel · 2018-07-27T04:40:34Z

doc/source/whatsnew/v0.24.0.txt

@@ -475,7 +479,7 @@ Numeric
 - Bug in :class:`Series` ``__rmatmul__`` doesn't support matrix vector multiplication (:issue:`21530`)
 - Bug in :func:`factorize` fails with read-only array (:issue:`12813`)
 - Fixed bug in :func:`unique` handled signed zeros inconsistently: for some inputs 0.0 and -0.0 were treated as equal and for some inputs as different. Now they are treated as equal for all inputs (:issue:`21866`)
-
+- Bug in :class:`Series` comparison against datetime-like scalars and arrays (:issue:`22074`)


Not where this line was intended, will move.

I take it back; this is where it belongs.

gfyoung · 2018-07-27T05:10:00Z

pandas/tests/indexes/datetimes/test_arithmetic.py

+        with pytest.raises(TypeError):
+            rng > other
+        with pytest.raises(TypeError):
+            rng >= other


Anything worth checking error message-wise?

These are pretty scattershot at the moment. I'll give some thought to how that can be addressed.

codecov · 2018-07-27T18:59:44Z

Codecov Report

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

@@            Coverage Diff             @@
##           master   #22074      +/-   ##
==========================================
- Coverage   92.07%   92.06%   -0.01%     
==========================================
  Files         170      170              
  Lines       50688    50701      +13     
==========================================
+ Hits        46671    46679       +8     
- Misses       4017     4022       +5

Flag	Coverage Δ
#multiple	`90.47% <100%> (-0.01%)`	⬇️
#single	`42.33% <45.83%> (+0.01%)`	⬆️

Impacted Files	Coverage Δ
pandas/core/ops.py	`96.15% <100%> (+0.03%)`	⬆️
pandas/core/arrays/datetimes.py	`95.52% <100%> (+0.06%)`	⬆️
pandas/core/arrays/datetimelike.py	`94.02% <0%> (-1.04%)`	⬇️
pandas/core/indexes/datetimes.py	`95.54% <0%> (-0.14%)`	⬇️

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 26b3e7d...3ac9102. Read the comment docs.

jreback

nice fixtures. need to narrow down existing issues which it closes.

jreback · 2018-07-28T12:56:43Z

pandas/core/arrays/datetimes.py

@@ -99,31 +100,40 @@ def wrapper(self, other):
        meth = getattr(dtl.DatetimeLikeArrayMixin, opname)

        if isinstance(other, (datetime, np.datetime64, compat.string_types)):
-            if isinstance(other, datetime):
+            if isinstance(other, (datetime, np.datetime64)):


since we do this all over the place, thing about making a type for this (for in internal use only), and/or maybe a is_datetime_scalar or something, just so we are consistent

I've got a branch going looking at core.dtypes.common (inspired by noticing the footgun status if is_datetimelike), will look at this there so I can get all the places it is used in one swoop.

jreback · 2018-07-28T12:57:27Z

pandas/core/arrays/datetimes.py

@@ -152,6 +162,10 @@ class DatetimeArrayMixin(dtl.DatetimeLikeArrayMixin):
                 'is_year_end', 'is_leap_year']
    _object_ops = ['weekday_name', 'freq', 'tz']

+    # dummy attribute so that datetime.__eq__(DatetimeArray) defers


huh? why don't you just implement and raise NotIMplemented?

This is is a quirk of the stdlib datetime implementation. datetime == other will return NotImplemented instead of raising TypeError iff other has a timetuple attribute.

jreback · 2018-07-28T12:58:08Z

pandas/tests/indexes/datetimes/test_arithmetic.py

@@ -275,6 +275,20 @@ def test_comparison_tzawareness_compat(self, op):
        with pytest.raises(TypeError):
            op(ts, dz)

+    @pytest.mark.parametrize('op', [operator.eq, operator.ne,


pretty sure we have fixtures for this

There's a fixture for the names ["__eq__", ...]. This will go on the list of things I'd like to standardize after collecting arithmetic tests in one place.

jreback · 2018-07-28T13:18:38Z

pls rebase

jbrockmendel · 2018-07-28T16:39:19Z

need to narrow down existing issues which it closes.

AFAICT #7830 is the only issue this closes on its own. BUT after this is done we can make more DataFrame ops operate column-wise (orthogonal to #22019 because DataFrame ops are a hive of scum and villainy), and that closes a ton of issues.

update That follow-up will be made much less verbose if preceeded by #22068.

jreback · 2018-07-29T15:53:42Z

AFAICT #7830 is the only issue this closes on its own. BUT after this is done we can make more DataFrame ops operate column-wise (orthogonal to #22019 because DataFrame ops are a hive of scum and villainy), and that closes a ton of issues.

update That follow-up will be made much less verbose if preceeded by #22068.

you wnt to merge this first then #22068 ?

jbrockmendel · 2018-07-29T17:05:44Z

This and #22068 should be orthogonal, either order is fine.

jreback · 2018-07-31T13:06:02Z

doc/source/whatsnew/v0.24.0.txt

@@ -496,6 +496,9 @@ Datetimelike
 - Fixed bug where two :class:`DateOffset` objects with different ``normalize`` attributes could evaluate as equal (:issue:`21404`)
 - Fixed bug where :meth:`Timestamp.resolution` incorrectly returned 1-microsecond ``timedelta`` instead of 1-nanosecond :class:`Timedelta` (:issue:`21336`,:issue:`21365`)
 - Bug in :func:`to_datetime` that did not consistently return an :class:`Index` when ``box=True`` was specified (:issue:`21864`)
+- Bug in :class:`DatetimeIndex` comparisons where string comparisons incorrectly raises ``TypeError`` (:issue:`22074`)


should one be marked #7830 ?

jreback · 2018-07-31T13:06:49Z

tiny comment, not sure if worth changing

jbrockmendel · 2018-07-31T15:58:23Z

tiny comment, not sure if worth changing

Let's roll that into one of the cleanup-followups. I'm eager to do the fix-all-the-bugs one that comes after this.

jreback · 2018-08-01T09:19:53Z

thxs

jbrockmendel added 6 commits July 26, 2018 19:25

fix tzawareness compat with np.datetime64

da6878a

Fix DTI comparisons against invalid strings

ba6fdd0

Whatsnew, de-duplicate test

6e92dc2

Fix DTI comparisons against TDI

16202d5

Fix DTI comparisons against object-dtype

c4ab98c

Add GH references

92b74ec

jbrockmendel commented Jul 27, 2018

View reviewed changes

fix incorrect tests, update whatsnew

23aa2a0

jbrockmendel commented Jul 27, 2018

View reviewed changes

gfyoung added Bug Datetime Datetime data dtype labels Jul 27, 2018

gfyoung reviewed Jul 27, 2018

View reviewed changes

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

b6bd737

jreback requested changes Jul 28, 2018

View reviewed changes

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

4b46d6b

jbrockmendel mentioned this pull request Jul 28, 2018

Breaking change multiplication bool and int #22047

Closed

jbrockmendel mentioned this pull request Jul 29, 2018

Centralize m8[ns] Arithmetic Tests #22118

Merged

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

3ac9102

jbrockmendel mentioned this pull request Jul 30, 2018

[DEPR] Assorted footguns in dtype checks #22137

Closed

6 tasks

jreback reviewed Jul 31, 2018

View reviewed changes

jreback added this to the 0.24.0 milestone Jul 31, 2018

jreback approved these changes Jul 31, 2018

View reviewed changes

jbrockmendel mentioned this pull request Aug 1, 2018

~Finish collecting m8[ns] tests, start collecting division by zero tests #22153

Merged

jreback merged commit 8d5c51b into pandas-dev:master Aug 1, 2018

dberenbaum pushed a commit to dberenbaum/pandas that referenced this pull request Aug 3, 2018

[Bug] Fix various DatetimeIndex comparison bugs (pandas-dev#22074)

4ca3d46

jbrockmendel deleted the dtcmp branch August 8, 2018 15:52

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

[Bug] Fix various DatetimeIndex comparison bugs (pandas-dev#22074)

b83f8d8

jbrockmendel mentioned this pull request Oct 24, 2018

DataFrame vs Series vs Index arithmetic Roundup #18824

Closed

59 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Bug] Fix various DatetimeIndex comparison bugs #22074

[Bug] Fix various DatetimeIndex comparison bugs #22074

jbrockmendel commented Jul 27, 2018 •

edited

Loading

jbrockmendel Jul 27, 2018

jbrockmendel Jul 27, 2018

jbrockmendel Jul 27, 2018

gfyoung Jul 27, 2018

jbrockmendel Jul 27, 2018

codecov bot commented Jul 27, 2018 •

edited

Loading

jreback left a comment

jreback Jul 28, 2018

jbrockmendel Jul 28, 2018

jreback Jul 28, 2018

jbrockmendel Jul 28, 2018

jreback Jul 28, 2018

jbrockmendel Jul 28, 2018

jreback commented Jul 28, 2018

jbrockmendel commented Jul 28, 2018 •

edited

Loading

jreback commented Jul 29, 2018

jbrockmendel commented Jul 29, 2018

jreback Jul 31, 2018

jreback commented Jul 31, 2018

jbrockmendel commented Jul 31, 2018

jreback commented Aug 1, 2018

[Bug] Fix various DatetimeIndex comparison bugs #22074

[Bug] Fix various DatetimeIndex comparison bugs #22074

Conversation

jbrockmendel commented Jul 27, 2018 • edited Loading

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 Jul 27, 2018 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Jul 28, 2018

jbrockmendel commented Jul 28, 2018 • edited Loading

jreback commented Jul 29, 2018

jbrockmendel commented Jul 29, 2018

Choose a reason for hiding this comment

jreback commented Jul 31, 2018

jbrockmendel commented Jul 31, 2018

jreback commented Aug 1, 2018

jbrockmendel commented Jul 27, 2018 •

edited

Loading

codecov bot commented Jul 27, 2018 •

edited

Loading

jbrockmendel commented Jul 28, 2018 •

edited

Loading