ENH date_range accepts timedelta as freq #6318
Conversation
|
Can you also update the date_range docstring? |
|
you need to fail this if u r on numpy < 1.7 (use pd._np_under_p17); raise that it is not supported (see pd.to_timedelta) for what to do and skip tests if the same (np.timdelta64 has a diff signature under 1.6) |
|
whoops, I had made docstring, but forgot to save that file. Thanks, will fix up for <1.7. |
|
looks like datetime timedelta don't have a total_seconds attribute in 2.6. |
|
yep see my comment in the pr - just pull out the days, seconds and microseconds directly |
|
@hayd any thoughts about not having this always return micro/nano and instead a higher freq if possible? |
|
Yeah, will have another look at this tomorrow. It begs question should we always be simplifying ticks? |
|
hmm offhand I'll say that the resample code doesn't care whether it's simplified or not (and doesn't make a difference) - can u give a look. see to see if it matters? |
|
resample won't care, will be in the freq repr for the index... datetime.timedelta does some simplifying think we probably should. sigh, to micros being timedeltas smallest unit, this means the delta attribute does different things for nano and everything else: In [21]: Nano(1000) == Micro(1) Will fix as part of this... |
|
@hayd ok with merging this like this (after rebase)....then maybe create another issue about the simplification stuff? |
|
yea, I refactored this a little already (but not pushed and away atm) so future change should be straightforward. Will do next week. |
|
ping! |
|
ping |
|
moving to 0.15 |
|
related #5963. |
| from pandas import _np_version_under1p7 | ||
| if _np_version_under1p7: | ||
| raise nose.SkipTest("date_range with freq timedelta " | ||
| "not supported numpy < 1.7") |
There was a problem hiding this comment.
this may not need to be skipped
|
I think the pending part of this was comparison of Tickers (previously |
|
if u want to go down the rabbit hole |
| @@ -2783,8 +2783,7 @@ def test_should_cache_week_month(self): | |||
|
|
|||
| def test_all_cacheableoffsets(self): | |||
| for subclass in get_all_subclasses(CacheableOffset): | |||
| if subclass.__name__[0] == "_" \ | |||
| or subclass in TestCaching.no_simple_ctr: | |||
There was a problem hiding this comment.
This attribute doesn't seem to exist anywhere... removing looks like it fixes.
|
Note: docstring doesn't require changing as it already suggests that a timedelta should work. |
|
pls change release notes to v0.14.1 thanks! |
|
added and green (on my travis). |
| @@ -387,6 +398,38 @@ def get_legacy_offset_name(offset): | |||
| name = offset.name | |||
| return _legacy_reverse_map.get(name, name) | |||
|
|
|||
|
|
|||
| def _simplify_offset(offset): | |||
There was a problem hiding this comment.
this is completely internal, yes? any reason to NOT always simplify? (e.g. in to_offset)
There was a problem hiding this comment.
to_offset uses offsets._delta_to_tick which also does this simplifying in a similar operation (delta to offset)...
...Ah ha, but to_offset(an_offset) atm is just an_offset, are you suggesting it should return _simplify_offset(an_offset)? This seems reasonable/in line with rest of function.
|
I think it makes sense to simply if its unambiguous |
|
agree, done. Also removed some "horrible-ness". |
FIX compare ns Ticker with others FIX remove reference to no_simple_ctr (nowhere else) DOC move to 0.14.1
|
Hmmm I thought this had already been merged in. |
|
rebase this, and I think you have a 2.6 failing issue |
|
@hayd ? |
|
@hayd let's revist |
|
Sigh, it's only 2.6 that's at issue here (should probably remove the |
|
@hayd ? |
|
@hayd ? |
|
closing as stale. @hayd if you'd like to update, pls reopen |
fixes #6307