Warn on ndarray[int] // timedelta #21036
Conversation
Closes pandas-dev#19761. ```python In [2]: pd.DatetimeIndex(['1931', '1970', '2017']).view('i8') // pd.Timedelta(1, unit='s') pandas-dev/bin/ipython:1: FutureWarning: Floor division between integer array and Timedelta is deprecated. Use 'array // timedelta.value' instead. Out[2]: array([-1230768000, 0, 1483228800]) ```
TomAugspurger
added
Numeric Operations
| elif other.dtype.kind == 'i': | ||
| # Backwards compatibility | ||
| # GH-19761 | ||
| msg = textwrap.dedent("""\ |
There was a problem hiding this comment.
you don't need the textwrap dedent if you put the string over multiple lines with implicit line continuation and string concatentation?
msg = (" ... "
" ... ")
(I would personally find that a bit cleaner (also don't need to \), but no big deal)
There was a problem hiding this comment.
I've been burned so many times by implicit string concatenation across lines that I try to always avoid it in the hope that it'll be removed in Python 4 :)
doc/source/timeseries.rst
Outdated
| @@ -308,7 +308,7 @@ We convert the ``DatetimeIndex`` to an ``int64`` array, then divide by the conve | |||
|
|
|||
| .. ipython:: python | |||
|
|
|||
| stamps.view('int64') // pd.Timedelta(1, unit='s') | |||
| stamps.view('int64') // pd.Timedelta(1, unit='s').value | |||
|
|
|||
There was a problem hiding this comment.
I would prob change this example to
In [11]: i
Out[11]: DatetimeIndex(['1931-01-01', '1970-01-01', '2017-01-01'], dtype='datetime64[ns]', freq=None)
In [12]: (i-pd.Timestamp(0)) // pd.Timedelta(1, unit='s')
Out[12]: Int64Index([-1230768000, 0, 1483228800], dtype='int64')
There was a problem hiding this comment.
That makes sense for now (till to_epoch is ready).
Codecov Report
@@ Coverage Diff @@
## master #21036 +/- ##
=======================================
Coverage 91.82% 91.82%
=======================================
Files 153 153
Lines 49502 49502
=======================================
Hits 45457 45457
Misses 4045 4045
Continue to review full report at Codecov.
|
|
thanks! |
| # GH-19761 | ||
| msg = textwrap.dedent("""\ | ||
| Floor division between integer array and Timedelta is | ||
| deprecated. Use 'array // timedelta.value' instead. |
There was a problem hiding this comment.
Since you updated the example in the docs, maybe we should reflect this as well in the deprecation message?
|
The maybe tricky thing there is that not all ndarray[int] // timedelta
operations are necessarily for epoch conversions. They would be the
majority though...
…On Tue, May 15, 2018 at 8:06 AM, Joris Van den Bossche < ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In pandas/_libs/tslibs/timedeltas.pyx
<#21036 (comment)>:
> @@ -1188,6 +1190,15 @@ class Timedelta(_Timedelta):
if other.dtype.kind == 'm':
# also timedelta-like
return _broadcast_floordiv_td64(self.value, other, _rfloordiv)
+ elif other.dtype.kind == 'i':
+ # Backwards compatibility
+ # GH-19761
+ msg = textwrap.dedent("""\
+ Floor division between integer array and Timedelta is
+ deprecated. Use 'array // timedelta.value' instead.
Since you updated the example in the docs, maybe we should reflect this as
well in the deprecation message?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#21036 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABQHIpnZJXikD1ccI2FzlnxTxW-tMSEtks5tytLSgaJpZM4T-UP4>
.
|
|
I don't know who else would be doing that :-) But it's true that it's not necessarily the case. I can do a PR to add that explanation in addition to the current one if you like. |
|
That'd be great. Tracking down some dask issues to verify we don't have any
other regressions right now, so we've got some time.
…On Tue, May 15, 2018 at 9:31 AM, Joris Van den Bossche < ***@***.***> wrote:
I don't know who else would be doing that :-) But it's true that it's not
necessarily the case. I can do a PR to add that explanation in addition to
the current one if you like.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#21036 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABQHIi7XS2ASNaCSIUpCRLQ6WHn_nsa-ks5tyubdgaJpZM4T-UP4>
.
|
|
Should users also get warned on timedelta // int, which happens with timedelta % int too? Maybe the warning should pop up everytime a number is implicitly converted to a timestamp, as the current behavior does not seem very consistent with addition for example. |
Closes #19761.
Long-term, we'll recommend using
to_epochfor the case where people are doing this to do conversion to unix epoch. But #14772 has a few design issues that will take some time to discuss. I think we should just recommend.valuefor now.