ENH: Allow aggregate numeric operations on timedelta64.

[danielballan]

closes #5724

Currently, timedelta64 columns can't be used in aggregate group operations. They are quietly dropped or, if no other columns are present, an exception is raised:

In [2]: df
Out[2]: 
     A        E
0  foo 00:00:01
1  bar 00:00:02
2  foo 00:00:03
3  bar 00:00:04


In [3]: df.groupby('A').mean()
Out[3]: 
(...)
DataError: No numeric types to aggregate

But any operation that works on numerical data is also well defined on timedeltas. This PR relaxes the restriction on those methods designated "numeric only" to accept numeric dtypes and timedelta64. I include a unit test with one simple case each for DataFrameGroupBy and SeriesGroupBy.

In [2]: df.groupby('A').mean()
Out[2]: 

           E
A           
bar 00:00:03
foo 00:00:02

[jreback]

FYI their is an issue for adding timedelta support to groupby - see if u can find it

also this needs a fair amount of tests as trit are many paths thru groupby

basically need to mimic most of the datetime64 type tests

[danielballan]

Have we established some pattern for skipping timedelta tests for numpy < 1.7?

Also, while I'm here, I'll fix the English in this error....

ValueError: to_timedelta is not support for numpy < 1.7

[jreback]

numpy timedelta is thoroughly broken < 1.7

[cancan101]

See for example: https://github.com/pydata/pandas/blob/master/pandas/tseries/tests/test_timedeltas.py#L21

[jreback]

@danielballan hows' this coming?

[danielballan]

A quick, perhaps obvious, thought: this is not exactly analogous to what was done for datetime. For example, summing datetimes is (correctly) not defined but summing timedeltas should be.

But many of the tests will be analogous. I have to put this aside until next week, but I can pick it up by the beginning of May. If someone wants to tap in before then, feel free to speak up.

[jreback]

gr8 thanks

would like like to get some minimal set at least in for 0.14

as I agree it's a bit nontrivial
so test away!

[jreback]

@danielballan can you rebase, add a couple more tests, and release note / v0.14.0.txt ?

[keir]

I am working on finishing this patch, and I'm not convinced this is the right approach. What about making timedeltas considered a numeric type? Otherwise we're going to have to update all locations where there are is_numeric checks.

I suppose this opens the question about what "is_numeric" means.

In particular, the current Pandas master does

def _cython_agg_blocks(self, how, numeric_only=True):
    ...
    if numeric_only:
        data = data.get_numeric_data(copy=False)

I just started using Pandas today, so it's possible the original patch is the right approach.

[keir]

@jreback

[danielballan]

I considered lumping time deltas in with is_numeric, but I decided it was
clearer, more readable this way. Still up for debate though.

On Tuesday, April 29, 2014, Keir Mierle [email protected] wrote:

@jreback https://github.com/jreback

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/6884#issuecomment-41642641
.

[danielballan]

Rebased.

I can add tests this week, but I also welcome any contributions that @keir has to offer. FYI, @keir, as I revisit this, I find one place where we check is_numeric in a way that excludes timedeltas: above the call to ensure_float. This is why I made them separate.

[keir]

@danielballan, I am operating off trunk, which seems to have changed one of the functions (_cython_agg_blocks). I ported the code tried my approach to make timedelta is_numeric, but I am now not convinced of the approach. Do you want to update to master?

[danielballan]

OK. I updated to master this morning. See the latest version of the commit. (N.B. The tests are broken; this is a work-in-progress commit.)

[jreback]

@danielballan how's this coming?

[danielballan]

@jreback If I can push through this this weekend, still time for 0.14? Actually maybe better to wait for 0.14.1 so we can get #5986 also.

[jreback]

well I want to get a RC1 out, but if you are ready after, then ok

[jreback]

FYI, I think #5986 for quantile has been fixed by @TomAugspurger very recently

[TomAugspurger]

I don't know if I tested timedeltas at all in quantile, just timestamps.

[keir]

I don't suggest merging this patch. I have a new, smaller, different patch that changes timedelta64 to be a numeric type. I will try to upload it later today but currently I have some production issues.

[danielballan]

@keir Does that approach create problems when com.ensure_float is called on timedelta64 data? I am eager to see it and build out tests. Maybe we are too late for 0.14, which is too bad, but I am eager to have it available on master regardless.

[jreback]

@danielballan 0.14.0 released (ann coming later today), but this could be on master/0.14.1 in any event

[jreback]

@danielballan can you rebase this?

[jreback]

@danielballan revisit? i'd like to get into 0.15

[danielballan]

@jreback I won't have time to touch this again until January, but at that point I'll be joining mpl's @tacaswell at Brookhaven, and I will have some solid time to contribute again.

[jreback]

@danielballan np, should be straight forward with the new Timedelta type in any event.

have fun, according to cc @ericdill you have LOTS of data!

[danielballan]

I was glad to see Timedelta go in. Yes, we have our work cut out for us!

[jreback]

@danielballan can you revisit / update

[danielballan]

OK, I'm back on the job. :- D

[gruen]

My sanity thanks you.

On Jan 18, 2015, at 4:57 PM, Dan Allan [email protected] wrote:

OK, I'm back on the job. :- D

—
Reply to this email directly or view it on GitHub.

[jreback]

closing pls reopen if/when updated

[rjp23]

Trying to work out why groupby was dropping my datetime columns and came across this. Did it never get fixed?

[TomAugspurger]

Looks like this PR went stale. Let us know if you want to pick it up @rjp23.

…

On Sun, Jun 9, 2019 at 10:58 PM rjp23 ***@***.***> wrote: Trying to work out why groupby was dropping my datetime columns and came across this. Did it never get fixed? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6884?email_source=notifications&email_token=AAKAOITYC6WMYKN2WA3RL7DPZXGPXA5CNFSM4AOJZLUKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXI3VQQ#issuecomment-500284098>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAKAOIWBUEKLA4TI2K5CIS3PZXGPXANCNFSM4AOJZLUA> .

[rjp23]

Well it should be fixed but I don't think I'm knowledgeable enough to do it.

[jazzlw]

This seems to have been fixed somewhere between version 1.3.4 and version 2.2.0 (two versions I had convenient to test) but I'm not sure when or how. If someone knows, can they put it here for people who end up here confused in the future, as I did?