TST: Groupby filter tests involved len, closing #4447

[danielballan]

closed #4447 (tests only, so no release notes necessary)

...which was apparently fixed by #4657. More tests looking for shape-related bugs in filter would be nice. So far I have no discovered any.

[jreback]

maybe add some more functions here? that are mostly errors

e.g. return a weird shape, return different dtypes, raise an error

def f(x):
     if len(x) > 1:
           raise TypeError():
     return True

def f(x):
      return np.nan

def f(x):
       return x

def f(x):
       return x == 1

[danielballan]

Thanks for those examples. In this WIP commit, the commented-out tests are not raising as they should, so some work is needed on filter itself. Also, there is some inconsistency between ValueError/TypeError depending on DataFrame/Series. Will keeping working...

[jreback]

yep....assume the user doesn't know what they are doing and don't read the docs :) then try to do something reasonable (including raising a helpful message that filter must return a boolean value!)

[jreback]

@danielballan ready to go on this?

[danielballan]

Nearly. First, a question: if filter returns a non-boolean value, should we proceed to evaluate its truth? (As far as I understand, this calls __nonzero__ in 2.x, __bool__ in 3.x.) Specifcally, should grouped.filter(lambda x: 1) raise or behave like grouped.filter(lambda x: True)? What about np.nan, which also evaluates True? I lean toward raising on np.nan but tolerating 1. Opinions?

[jtratner]

I'd expect it to accept anything bool-like, potentially with special case for nan being equivalent to False (and producing ndarrays with filter will raise as expected)

[danielballan]

OK, assuming Travis does not object, this is good to go.

[jreback]

looks good

at some future point an example for docs/cookbook which used a non-trivial filter

[jreback]

@jtratner @cpcloud comments?

[cpcloud]

should this and be &? can b be an array/frame/series?

[danielballan]

I think it has to be and in order to handle NaN, which we want to interpret as False. Thoughts, @cpcloud?

In [59]: np.nan & pd.notnull(np.nan)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-59-47e0df96d143> in <module>()
----> 1 np.nan & pd.notnull(np.nan)

TypeError: unsupported operand type(s) for &: 'float' and 'bool'

In [60]: np.nan and pd.notnull(np.nan)
Out[60]: False

[jreback]

@danielballan pls squash this down when you are ready

[danielballan]

Ready, pending discussion on & vs. and with cpcloud above.

[jreback]

this looks fine....@cpcloud

[jreback]

this is ok...merging