Skip to content
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

ENH: add to_offset method to Timedelta #9064

Closed
jreback opened this issue Dec 12, 2014 · 6 comments · Fixed by #9226
Closed

ENH: add to_offset method to Timedelta #9064

jreback opened this issue Dec 12, 2014 · 6 comments · Fixed by #9226
Labels
Dtype Conversions Unexpected or buggy dtype conversions Enhancement Frequency DateOffsets Timedelta Timedelta data type
Milestone

Comments

@jreback
Copy link
Contributor

jreback commented Dec 12, 2014

from SO

This works, but I think td.to_offset() makes sense

In [1]: td = Timedelta('1h')

In [2]: td
Out[2]: Timedelta('0 days 01:00:00')

In [7]: pd.DateOffset(hours=td.hours)
Out[7]: <DateOffset: kwds={'hours': 1}>
@jreback jreback added Enhancement Good as first PR Dtype Conversions Unexpected or buggy dtype conversions labels Dec 12, 2014
@jreback jreback added this to the 0.16.0 milestone Dec 12, 2014
@jreback jreback added Frequency DateOffsets Timedelta Timedelta data type labels Dec 12, 2014
@tvyomkesh
Copy link
Contributor

Does this look like an acceptable ENH (as suggested in SO) or is it expected to introduce a to_offset() method in timedelta which returns offsets.Second?

diff --git a/pandas/tseries/frequencies.py b/pandas/tseries/frequencies.py
index 54b29b1..8694436 100644
--- a/pandas/tseries/frequencies.py
+++ b/pandas/tseries/frequencies.py
@@ -1,4 +1,4 @@
-from datetime import datetime
+from datetime import datetime,timedelta
 from pandas.compat import range, long, zip
 from pandas import compat
 import re
@@ -298,6 +298,10 @@ def to_offset(freqstr):
             name, stride = stride, name
         name, _ = _base_and_stride(name)
         delta = get_offset(name) * stride
+
+    elif isinstance(freqstr, timedelta):
+        delta = offsets.Second(freqstr.total_seconds())
+
     else:
         delta = None
         stride_sign = None

@jreback
Copy link
Contributor Author

jreback commented Jan 12, 2015

create a test that fails w/o the fix and passes with

@tvyomkesh
Copy link
Contributor

Done. Created pull request #9226. Thanks.

@tvyomkesh
Copy link
Contributor

CI build failed.

ERROR: pandas.tseries.tests.test_frequencies.test_to_offset_timedelta

Traceback (most recent call last):
File "/home/travis/miniconda/envs/pandas/lib/python2.6/site-packages/nose/case.py", line 197, in runTest
self.test(*self.arg)
File "/home/travis/build/pydata/pandas/pandas/tseries/tests/test_frequencies.py", line 86, in test_to_offset_timedelta
result = frequencies.to_offset(td)
File "/home/travis/build/pydata/pandas/pandas/tseries/frequencies.py", line 303, in to_offset
delta = offsets.Second(freqstr.total_seconds())

AttributeError: 'datetime.timedelta' object has no attribute 'total_seconds'

I will need to revisit and add existence check for method etc.

@jorisvandenbossche
Copy link
Member

@tvyomkesh the reason is that total_seconds was only added in python 2.7/3.2 (it only fails the tests for 2.6)
But we better continue the discussion on the PR

@tvyomkesh
Copy link
Contributor

@jorisvandenbossche for now I have fixed this by introducing a check if the method is available or not (https://github.com/pydata/pandas/pull/9226/files). For older versions which do not have the method, it will use the equivalent calculation mentioned in the Python 2.7 docs (https://docs.python.org/2/library/datetime.html#datetime.timedelta.total_seconds).
Wondering if it would be a good idea to also wrap it up with try/catch. Happy to discuss more.

jreback added a commit that referenced this issue Jan 18, 2015
ENH: add to_offset method to Timedelta #9064
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dtype Conversions Unexpected or buggy dtype conversions Enhancement Frequency DateOffsets Timedelta Timedelta data type
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants