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: GH3371 support timedelta fillna #4684

Merged
merged 3 commits into from
Sep 8, 2013

Conversation

jreback
Copy link
Contributor

@jreback jreback commented Aug 27, 2013

closes #3371

  • ENH: GH3371 support timedelta fillna
  • BUG: add support for datetime64 ffill/bfill
  • INT: add TimeDeltaBlock support in internals
In [3]: s = Series([Timestamp('20130101'),Timestamp('20130101'),
                  Timestamp('20130102'),Timestamp('20130103 9:01:01')])

In [4]: td = s.diff()

In [5]: td
Out[5]: 
0                NaT
1           00:00:00
2   1 days, 00:00:00
3   1 days, 09:01:01
dtype: timedelta64[ns]

In [6]: td.fillna(0)
Out[6]: 
0           00:00:00
1           00:00:00
2   1 days, 00:00:00
3   1 days, 09:01:01
dtype: timedelta64[ns]

In [7]: from datetime import timedelta

In [8]: td.fillna(timedelta(days=3,seconds=5))
Out[8]: 
0   3 days, 00:00:05
1           00:00:00
2   1 days, 00:00:00
3   1 days, 09:01:01
dtype: timedelta64[ns]

ffill

In [9]: td[2] = np.nan

In [10]: td
Out[10]: 
0                NaT
1           00:00:00
2                NaT
3   1 days, 09:01:01
dtype: timedelta64[ns]

In [11]: td.ffill()
Out[11]: 
0                NaT
1           00:00:00
2           00:00:00
3   1 days, 09:01:01
dtype: timedelta64[ns]

@jreback
Copy link
Contributor Author

jreback commented Sep 8, 2013

@cpcloud numpy 1.6.1 compat shall be conquered! (for timedelta)....what a mess they made :<

@@ -705,6 +705,59 @@ def diff(arr, n, axis=0):
return out_arr


def _coerce_scalar_to_timedelta_type(r):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is so insane 😄

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a suggestion - it might be easier to read if you separated the under 1.7 handling, like this: (I'm pretty sure this does the same thing as the current function). To me, this also makes it easier to see that under 1.7 it returns a timedelta and above 1.7 it returns a timedelta64 type.

def _coerce_scalar_to_timedelta_type(r):
    # kludgy here until we have a timedelta scalar
    # handle the numpy < 1.7 case

    if is_integer(r):
        r = timedelta(microseconds=r/1000)

    if _np_version_under1p7:
        if not isinstance(r, timedelta):
            raise AssertionError("Invalid type for timedelta scalar: %s" % type(r))
        return r

    if isinstance(r, timedelta):
        r = np.timedelta64(r)
    elif not isinstance(r, np.timedelta64):
        raise AssertionError("Invalid type for timedelta scalar: %s" % type(r))
    return r.astype('timedelta64[ns]')

@jreback
Copy link
Contributor Author

jreback commented Sep 8, 2013

nope that should never even be user visible (the function or the exception)
in reality can just return the value but want it to raise if something slips thru

@jtratner
Copy link
Contributor

jtratner commented Sep 8, 2013

awesome - just wanted to check. You should add a pragma: no cover if you think the assertions should never happen... (that's what I've been trying to do throughout)

return x

if self.ndim == 1:
if obj.dtype == 'm8[us]':
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't you just slim this line and the next down to obj = f(obj)? (otherwise, you're just repeating that type check twice)....

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep could fix that up

@cpcloud
Copy link
Member

cpcloud commented Sep 8, 2013

@jtratner timedelta64 and datetime64 have extraordinarily poor support in numpy 1.6. For example, microsecond units are enforced without regard to...well, anything.

It says here that units are not converted correctly 😒, which has left @jreback and myself looking like this 😡 on several occasions.

Differences in np 1.6 and 1.7 datetime64/timedelta64

@jtratner
Copy link
Contributor

jtratner commented Sep 8, 2013

@cpcloud that's crazy.

API: exclude non-numerics if mixed types in _reduce operations

BUG: timedelta fixes

CLN: small cleaning in nanops.py

BUG: allow _reduce to call .apply for certain operations when the who block fails
     via a reduce exception
@jreback
Copy link
Contributor Author

jreback commented Sep 8, 2013

finally got this to pass.....3.2 with 1.6 is of course different in its handing of timedeltas than 2.7 with 1.6....oh numpy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG/ENH: timedelta fillna should work intuitively
3 participants