EHN: Add filter methods to SeriesGroupBy, DataFrameGroupBy GH919
State: closed

closes #919

I have been using Wes' workaround (see #919) for filtering groups. Finally, for brevity's sake, I wrote a real filter method. In the one simple case I checked, it performs faster than the workaround.

On a small (~10) Series:

```
In [7]: %timeit grouped.filter(lambda x: x.mean() > 10)  # my method
1000 loops, best of 3: 346 us per loop

In [8]: %timeit grouped.obj[grouped.transform(lambda x: x.mean() > 10)]  # workaround
1000 loops, best of 3: 462 us per loop
```

On a large (1000000) Series:

```
In [18]: %timeit grouped.filter(lambda x: x.mean() > 0) # my method
1 loops, best of 3: 213 ms per loop

In [19]: %timeit grouped.obj[grouped.transform(lambda x: x.mean() > 0)]
1 loops, best of 3: 696 ms per loop
```

This PR only handles Series, and I included one simple test. If I am on the right track, I'll write one for DataFrames also and write additional tests. If this is a job for Cython, I'm out of my depth, but I think numpy is sufficient. Does this look OK?


Labels:

Comments:
this looks ok. I don't think you need to cythonize at all. If you could add for DataFrame and some more tests would be great

@wesm ?

To avoid confusion with the [DataFrame method .filter](https://github.com/pydata/pandas/issues/1976), should we called this method `.having`, in a nod to the SQL usage, GROUP BY ... HAVING?

Then again, for users uninitiated in SQL, it might not be as self-evident what `.having` is meant for. Thoughts?

this is a groupby method, so there isn't a conflict with DataFrame per se, and I kind of like the nomeclature, it says, hey filter out these objs before you group....

(and I am fundamentally biased again SQL in any event :))

@wesm @y-p ?

That last PR came with a test for a single-column DataFrame and a tests for a multi-column DataFrame, where the filtering condition considers both columns. I will add more tests for thoroughness, but this looks OK so far.

I'm confused why the new Travis build failed. The error looks wholly unrelated to my changes, something about the wrong version of PyTables....

rebase on master; was a small issue with pytables versioning when 3.0.0 came out

Just chiming in to say that I'm very excited to see this merged!
+1

@danielballan looks like you have some merge commits in here....

using I do this to avoid that....

```
git pull (in master)
git br my-branch --set-upstream master
git checkout my-branch
git pull --rebase
```

you can get rid of them by (after git pull in master)

```
git checkout my-branch
git rebase -i <starting-comit>
```

then remove the offending commits (just delete them)

@danielballan also I would add some tests where you filter out some groups entirely (and thereby return nan for those groups, I think; could also drop them...but nan prob more consistent)

@jreback [Edit: Wrong Jeff, sorry.] The point of this PR is to filter out groups entirely or not at all, based on a criterion applied the each group as a unit. It does not filter within groups.

As for returning NaNs vs. dropping, most use cases I have seen call for dropping. I could add a drop keyword to provide a choice.

@danielballan I like have `drop=True` as a kw to filter, meaning don't return the invalid groups, if False then return na for those groups

Perhaps `dropna` to be consistent with `DataFrame.stack`?

In this case, drop means "Drop the filtered groups," not exactly "Drop missing values." So I like `drop` a little better, but I'm game to be persuaded or overruled.

@danielballan yes, brain damanged today.... @phobson +1 on `dropna`

@danielballan also....pls add some tests where you have a mixed frame, and case where the filter condition raises

All suggestions implemented. That last commit (TST...) implements `dropna` as well. Perhaps it would be best if I rebase this into one ENH commit when it's all ready to go.

@danielballan 

a lot of the logic in transform (the slow/fast path stuff) is shared with filter
can u refactor that out to another function to avoid dup code?

also pls add some tests that aggregate afterwards (test a priori filtering versus your filtering)
also I didn't look close enough. but pls add a test that is essentially your top-level example (eg using Wes workaround)

I'll add a test that compares to Wes's workaround.

Can you explain more what you mean b y "tests that aggregate afterwards"? I have some tests that compare results to data that I filtered "by hand."

when I wrote that for some reason was thinking that `filter` would be part of a pipeline...e.g.

`df.groupby('A').filter(lambda x: x.mean()>0).apply('mean')`

but that is really equivalent to

```
res = df.groupby('A').apply('mean')
res[res>0]
```

but 2nd one is almost certainly faster and more intuitve 
the filter operation by definition is a transform

@wesm comments on this?

@danielballan I think this is ok for 0.11.1....
can you add an example in v0.11.1, enhancements, and release notes mention?

Great. I'm finishing one more batch of tests. Will then work on documenting after I've pushed them.

also an example of usage in the groupby.rst as well (could be same example in whatsnew if that works)..I usually write the main docs first then just copy them (unless they are 2 long)

I'm new to writing pandas documentation, but it looks like I got it right. I wrote up three common use cases and showed it working on a Series, on a DataFrame, and with `dropna=False`.

great.....looks like you need to rebase then resubmit....

top-section `Filration` -> `Filtration`

@danielballan fyi...even after 0.11.1 is released can still update docs, and wes only cutting a release candidate I think

OK. As far as I can tell this is good to merge.

thanks you very much!