BUG: DataFrame.where does not handle Series slice correctly (#10218)
State: closed

closes #10218 


Labels:
name: Bug
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: Needs Discussion
description: Requires discussion from core team before further action

Comments:
@mortada if you can have look w.r.t to my comments

@jreback I dug into `align()` but I'm not quite sure how to get that to work

do you mean changing `align()` so that in this particular case, `cond, _ = cond.align(self)` would turn `cond` from a `Series` into a `DataFrame`? 

@mortada I think we need to add a parameter to `.align`, maybe `broadcast=False` (so default is the same as before). if `broadcast==True`, then normalize to the higher of the ndims of the returned objects. kind of like [this](https://github.com/pydata/pandas/blob/master/pandas/core/panel.py#L1089)

@jreback added `broadcast` keyword for `align()`, please take a look. 

Woah, not sure this is a good idea. Pandas currently does broadcasting the other way (like numpy).

I do think this other broadcasting is more useful, but it _is_ an API change that should be discussed more holistically.

@shoyer not sure if u saw my prior comment
I asked @mortada to do it this way as it is rather general
this doesn't change the API at as this is turned off by default
only in very specific circumstances would you need/want this

My concern is that pandas (like NumPy) currently has a Series broadcast against the _rows_ of a DataFrame, similar to the situation for 1D/2D numpy arrays. So I don't think this is a good argument name, because here `broadcast=True` would imply broadcasting in a special custom way that isn't consistent with the remainder of pandas.

I don't really have a good alternative parameter name here. But I think it would be better simply to add support for `df.where(series)` then to call the parameter `broadcast`. Currently `where` doesn't support any type of broadcasting:

```
In [1]: import numpy as np

In [2]: import pandas as pd

In [3]: df = pd.DataFrame({'x': [0, 1], 'y': [2, 3]})

In [4]: s = pd.Series([True, False])

In [5]: df[s]
Out[5]:
   x  y
0  0  2

In [6]: df.where(s)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-6-537e7e30ec86> in <module>()
----> 1 df.where(s)

/Users/shoyer/miniconda/envs/santa-maria/lib/python2.7/site-packages/pandas/core/generic.pyc in where(self, cond, other, inplace, axis, level, try_cast, raise_on_error)
   3509
   3510         if isinstance(cond, NDFrame):
-> 3511             cond = cond.reindex(**self._construct_axes_dict())
   3512         else:
   3513             if not hasattr(cond, 'shape'):

/Users/shoyer/miniconda/envs/santa-maria/lib/python2.7/site-packages/pandas/core/series.pyc in reindex(self, index, **kwargs)
   2149     @Appender(generic._shared_docs['reindex'] % _shared_doc_kwargs)
   2150     def reindex(self, index=None, **kwargs):
-> 2151         return super(Series, self).reindex(index=index, **kwargs)
   2152
   2153     @Appender(generic._shared_docs['fillna'] % _shared_doc_kwargs)

/Users/shoyer/miniconda/envs/santa-maria/lib/python2.7/site-packages/pandas/core/generic.pyc in reindex(self, *args, **kwargs)
   1750         if kwargs:
   1751             raise TypeError('reindex() got an unexpected keyword '
-> 1752                     'argument "{0}"'.format(list(kwargs.keys())[0]))
   1753
   1754         self._consolidate_inplace()

TypeError: reindex() got an unexpected keyword argument "columns"
```

@shoyer if you see the above issue. This is exactly what this is try to address. But rather than a specific fix, I think a more general approach is warranted, maybe `broadcast_axis` might be a better name. I think this kind of behavior (manual broadcasting) should live in `.align` as that is the purpose; all other routines (including `.where` simply need call `.align` to get a compatible shape/axes).

I agree -- `broadcast_axis` in `align` seems like a reasonable way to handle this. Not entirely clear how that generalizes to interactions with panels but perhaps that's less important.

@shoyer oh I agree, just wanted to make a starting point and not bury align logic.

@mortada can you rename to `broadcast_axis` (and you will have to pass it explicity in the `.where`)

@shoyer @jreback renamed to `broadcast_axis` please take a look 

@mortada before you implement this, could we work out the desired semantics for `broadcast_axis`?

For example, I thought @jreback was suggesting `broadcast_axis` as an integer (since it has the name `axis`).

@shoyer that's a good point, the name does seem to suggest an `int` or `axis` name

yes I meant it should be None by default and be an axis (int/string)

@mortada can you update `broadcast_axis` to what we talked about above? e.g. an actual axis (int/string)

I think this also closes https://github.com/pydata/pandas/issues/9558 

that looks right, @mortada can you add tests for that as well.

@jreback sure will do

@jreback updated `broadcast_axis` and added to tests, please take a look 

@mortada I think just needs a rebase. can you added in `_shared_docs` for `.align` as well. thxs

@jreback one problem with using `_shared_docs` here is that `align()` is implemented in `generic.py` but the code and docstring for it are very `DataFrame` specific. So we wouldn't really be able to use the keyword substitutions 

@mortada ok, then I think what we need to do is make the doc-string in `generic.py` for > 2 dims
and put one in Series/Frame (if you are really fancy would do this with subsitutions, but whatever works).

@jreback sure, I've updated this to use `_shared_docs`

pls rebase, ping when green.

@jreback rebased and added `versionadded` to docstring

as for the docstring for `Panel` it is a bit tricky, because the implementation doesn't actually work with Panels yet. 

right, `Panel` does some other things. Ok, let's just raise a `NotImplementedError` in `Panel` then.
I don't think this should break anything.

It seems like broadcast_axis only makes sense for dataframes. Maybe it should be broadcast_axes instead? That would make sense for generalizing to panels.

On Fri, Aug 21, 2015 at 3:31 PM, Jeff Reback notifications@github.com
wrote:

> right, `Panel` does some other things. Ok, let's just raise a `NotImplementedError` in `Panel` then.
> 
> ## I don't think this should break anything.
> 
> Reply to this email directly or view it on GitHub:
> https://github.com/pydata/pandas/pull/10283#issuecomment-133580198

@shoyer indeed this is really only for `DataFrame`. With the `broadcast_axes` you are proposing, do you mean it is a `bool` or a list of multiple axes like `[1, 2]`?

Broadcast_axes would take a pair of tuples (or ints) indicating which axis from each argument to broadcast against the other.

On Sat, Aug 22, 2015 at 3:10 AM, Mortada Mehyar notifications@github.com
wrote:

> ## @shoyer indeed this is really only for `DataFrame`. With the `broadcast_axes` you are proposing, do you mean it is a `bool` or a list of multiple axes like `[1, 2]`?
> 
> Reply to this email directly or view it on GitHub:
> https://github.com/pydata/pandas/pull/10283#issuecomment-133622591

@mortada actually I think `broadcast_axis` is descriptive enough. and evenutally could be made to accept a list of axes (as we do with methods like `.apply`), but this is not necessary for now. When the panel fix is in, ping.