issue_comments

Hello @fujiisoup! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. :beers:

Comment last updated on December 24, 2018 at 12:51 Hours UTC

I moved MultiIndex.remove_unused_levels() to the inside of unstack. Now it is invoked only when unstack is called.

But this problem of .sel can be simply solved by manually raising a keyerror if the result of get_loc is size 0 array.

@shoyer

My inclination was just to copy what pandas does (which is only removing unused levels in unstack)

Thanks. I will take a look the source later.

Can you explain why matters for .sel()?

Selection of non-exsiting level variable should be KeyError, but it gives a 0-size index.

```python In [8]: ds = xr.DataArray(np.arange(40).reshape(8, 5), dims=['x', 'y'],
coords={'x': np.arange(8), 'y': np.arange(5)}).stack(xy=['x', 'y'])

In [9]: ds.isel(xy=ds['x'] < 4).sel(x=5) # should be KeyError Out[9]: <xarray.DataArray (y: 0)> array([], dtype=int64) Coordinates: * y (y) int64 ```

This issue does not matter only in unstack but also in sel (reindex also?).

Can you explain why matters for .sel()? I guess it slows down repeated indexing?

My inclination was just to copy what pandas does (which is only removing unused levels in unstack)

@shoyer

This issue does not matter only in unstack but also in sel (reindex also?). We can do it in both methods for efficiency, but I just prefered to do it in one place for the quick fix.

But what is the good place to do it actually? If we invoke this in sel, it would be again inefficient as this is called every time we did .sel.

Probably the best for the efficiency is to keep a flag for it in PandasIndexAdapter and do it in _get_item_with_mask and get_loc only if flag is raised?

Should we update levels after indexing or when stacking? Pandas does the later: ``` In [31]: import pandas as pd

In [32]: mindex = pd.MultiIndex.from_product([[1, 2, 3], ['a', 'b']])

In [33]: s = pd.Series(range(6), mindex)

In [34]: s.loc[:1].index Out[34]: MultiIndex(levels=[[1, 2, 3], ['a', 'b']], labels=[[0, 0], [0, 1]])

In [35]: s.loc[:1].unstack() Out[35]: a b 1 0 1 ```

The advantage of waiting until unstacking is that removing unused levels could be expensive compared to indexing: it runs in time O(L+N), where L is the size of the original level and N is the length of the new multi-index.