-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Revert change to comparison op with datetime.date objects #21361
Changes from all commits
95ec527
516cb89
d50fd6b
3394fdb
174c458
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,12 +10,52 @@ and bug fixes. We recommend that all users upgrade to this version. | |
:local: | ||
:backlinks: none | ||
|
||
|
||
.. _whatsnew_0231.fixed_regressions: | ||
|
||
Fixed Regressions | ||
~~~~~~~~~~~~~~~~~ | ||
|
||
**Comparing Series with datetime.date** | ||
|
||
We've reverted a 0.23.0 change to comparing a :class:`Series` holding datetimes and a ``datetime.date`` object (:issue:`21152`). | ||
In pandas 0.22 and earlier, comparing a Series holding datetimes and ``datetime.date`` objects would coerce the ``datetime.date`` to a datetime before comapring. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use double back ticks around Series (or |
||
This was inconsistent with Python, NumPy, and :class:`DatetimeIndex`, which never consider a datetime and ``datetime.date`` equal. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should double backtick on Python, NumPy |
||
|
||
In 0.23.0, we unified operations between DatetimeIndex and Series, and in the process changed comparisons between a Series of datetimes and ``datetime.date`` without warning. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. double backticks on DTI & Series |
||
|
||
We've temporarily restored the 0.22.0 behavior, so datetimes and dates may again compare equal, but restore the 0.23.0 behavior in a future release. | ||
|
||
To summarize, here's the behavior in 0.22.0, 0.23.0, 0.23.1: | ||
|
||
.. code-block:: python | ||
|
||
# 0.22.0... Silently coerce the datetime.date | ||
>>> Series(pd.date_range('2017', periods=2)) == datetime.date(2017, 1, 1) | ||
0 True | ||
1 False | ||
dtype: bool | ||
|
||
# 0.23.0... Do not coerce the datetime.date | ||
>>> Series(pd.date_range('2017', periods=2)) == datetime.date(2017, 1, 1) | ||
0 False | ||
1 False | ||
dtype: bool | ||
|
||
# 0.23.1... Coerce the datetime.date with a warning | ||
>>> Series(pd.date_range('2017', periods=2)) == datetime.date(2017, 1, 1) | ||
/bin/python:1: FutureWarning: Comparing Series of datetimes with 'datetime.date'. Currently, the | ||
'datetime.date' is coerced to a datetime. In the future pandas will | ||
not coerce, and the values not compare equal to the 'datetime.date'. | ||
To retain the current behavior, convert the 'datetime.date' to a | ||
datetime with 'pd.Timestamp'. | ||
#!/bin/python3 | ||
0 True | ||
1 False | ||
dtype: bool | ||
|
||
In addition, ordering comparisons will raise a ``TypeError`` in the future. | ||
|
||
**Other Fixes** | ||
|
||
- Reverted the ability of :func:`~DataFrame.to_sql` to perform multivalue | ||
inserts as this caused regression in certain cases (:issue:`21103`). | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,10 @@ | |
""" | ||
# necessary to enforce truediv in Python 2.X | ||
from __future__ import division | ||
import datetime | ||
import operator | ||
import textwrap | ||
import warnings | ||
|
||
import numpy as np | ||
import pandas as pd | ||
|
@@ -1197,8 +1200,35 @@ def wrapper(self, other, axis=None): | |
if is_datetime64_dtype(self) or is_datetime64tz_dtype(self): | ||
# Dispatch to DatetimeIndex to ensure identical | ||
# Series/Index behavior | ||
if (isinstance(other, datetime.date) and | ||
not isinstance(other, datetime.datetime)): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this check needed? (I would not expect it ever to be true if it is a datetime.date ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Apparently datetime.datetime subclasses datetime.date There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. rather than inlineing this, I think would be better to move to a module level function, maybe There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You mean just the new bit I added, right? I'm inclined to leave it here since we'll just be deleting it soon (is 0.24.0 too soon?). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Ah, only checked the other way around :-) Yeah, if this is only temporary code I would just leave it here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, is there an issue to fix this for 0.24.0? (can you add a TODO mention here) |
||
# https://github.com/pandas-dev/pandas/issues/21152 | ||
# Compatibility for difference between Series comparison w/ | ||
# datetime and date | ||
msg = ( | ||
"Comparing Series of datetimes with 'datetime.date'. " | ||
"Currently, the 'datetime.date' is coerced to a " | ||
"datetime. In the future pandas will not coerce, " | ||
"and {future}. " | ||
"To retain the current behavior, " | ||
"convert the 'datetime.date' to a datetime with " | ||
"'pd.Timestamp'." | ||
) | ||
|
||
if op in {operator.lt, operator.le, operator.gt, operator.ge}: | ||
future = "a TypeError will be raised" | ||
else: | ||
future = ( | ||
"'the values will not compare equal to the " | ||
"'datetime.date'" | ||
) | ||
msg = '\n'.join(textwrap.wrap(msg.format(future=future))) | ||
warnings.warn(msg, FutureWarning, stacklevel=2) | ||
other = pd.Timestamp(other) | ||
|
||
res_values = dispatch_to_index_op(op, self, other, | ||
pd.DatetimeIndex) | ||
|
||
return self._constructor(res_values, index=self.index, | ||
name=res_name) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use
date=datetime64[ns]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what you mean here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh was just saying that users talk about dtypes, rather than
Series
holding datetimes. no big deal really.