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

to_timedelta does not recognize 'h' and 'm' units #7611

Closed
vfilimonov opened this issue Jun 29, 2014 · 12 comments · Fixed by #7616
Closed

to_timedelta does not recognize 'h' and 'm' units #7611

vfilimonov opened this issue Jun 29, 2014 · 12 comments · Fixed by #7616
Labels
Bug Timedelta Timedelta data type
Milestone

Comments

@vfilimonov
Copy link
Contributor

'h' and 'm' units are not correctly recognized in to_timedelta:

print pd.to_timedelta(1,unit='D')
print pd.to_timedelta(1,unit='h')
print pd.to_timedelta(1,unit='m')
print pd.to_timedelta(1,unit='s')
print pd.to_timedelta(1,unit='ms')
print pd.to_timedelta(1,unit='us')
print pd.to_timedelta(1,unit='ns')

results in

86400000000000 nanoseconds
1 nanoseconds
1 nanoseconds
1000000000 nanoseconds
1000000 nanoseconds
1000 nanoseconds
1 nanoseconds

version: 0.14.0

Most likely related to Issue #6423

@jreback
Copy link
Contributor

jreback commented Jun 29, 2014

these are not valid numpy units (eg have to use H and M)
that said I thought this did work

@vfilimonov
Copy link
Contributor Author

It's the same issue for 'H' and 'M...

I picked 'h' and 'm' from to_timedelta's docstring:

unit : unit of the arg (D,h,m,s,ms,us,ns) denote the unit, which is an integer/float number

@jreback
Copy link
Contributor

jreback commented Jun 29, 2014

actually this is completely tested
what is your numpy version?

@vfilimonov
Copy link
Contributor Author

Sorry - forgot to tell: numpy 1.8.1

@vfilimonov
Copy link
Contributor Author

Great, @jreback, thanks a lot!

@jreback
Copy link
Contributor

jreback commented Jun 30, 2014

@vfilimonov thanks for reporting! was a bit buggy

@shoyer
Copy link
Member

shoyer commented Sep 5, 2014

@jreback Is there still a good reason for not handling h and m in pd.to_timedelta? It looks like pd.tslib.cast_from_unit can handle them now, and everything appears to work if I add them to regex in pandas.tseries.timedeltas.

@jreback
Copy link
Contributor

jreback commented Sep 5, 2014

#7616

fixed that - it was buggy before

@shoyer
Copy link
Member

shoyer commented Sep 5, 2014

@jreback Can we re-open this issue then? I'd like to submit a patch...

@jreback
Copy link
Contributor

jreback commented Sep 5, 2014

can u post what is not working?

@shoyer
Copy link
Member

shoyer commented Sep 5, 2014

>>> pd.to_timedelta('1h')
ValueError                                Traceback (most recent call last)
<ipython-input-80-e5635a653a40> in <module>()
----> 1 pd.to_timedelta('1h')

/Users/shoyer/miniconda/envs/xray-dev/lib/python2.7/site-packages/pandas/tseries/timedeltas.pyc in to_timedelta(arg, box, unit)
     67 
     68     # ...so it must be a scalar value. Return scalar.
---> 69     return _coerce_scalar_to_timedelta_type(arg, unit=unit)
     70 
     71 _unit_map = {

/Users/shoyer/miniconda/envs/xray-dev/lib/python2.7/site-packages/pandas/tseries/timedeltas.pyc in _coerce_scalar_to_timedelta_type(r, unit)
    117 
    118         # we are already converting to nanoseconds
--> 119         converter = _get_string_converter(r, unit=unit)
    120         r = converter()
    121         unit='ns'

/Users/shoyer/miniconda/envs/xray-dev/lib/python2.7/site-packages/pandas/tseries/timedeltas.pyc in _get_string_converter(r, unit)
    181 
    182     # no converter
--> 183     raise ValueError("cannot create timedelta string converter for [{0}]".format(r))
    184 
    185 def _possibly_cast_to_timedelta(value, coerce=True, dtype=None):

ValueError: cannot create timedelta string converter for [1h]

While we're at it, it would also be nice to include support all relevant long names (e.g., "hour", "min", "minute" etc). It is weird that only "day" and "days" work.

@jreback
Copy link
Contributor

jreback commented Sep 5, 2014

oh you are taking about the actual parser

ok (this issue covers things like
to_timedelta(1,unit='h') )

pls create a new issue for updates parsing. of strings then
the current impl was a bit quick and dirty so didn't cover all possibilities (although not hard I think)

prob should include some vbench / maybe cythonize at some point

it's a pretty limited regular format so shouldn't be that hard to make fast

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Timedelta Timedelta data type
Projects
None yet
3 participants