BUG: MultiIndex.get_level_values() replaces NA by another value (#5074)
State: closed

closes #5074

Do you really prefer using a mask? It seems to have a lower memory footprint but it requires some tricks for corner cases.

```
num = self._get_level_number(level)
unique_vals = self.levels[num]  # .values
labels = self.labels[num]
mask = labels == -1
if len(unique_vals) > 0:
    values = unique_vals.take(labels)
else:
    values = np.empty(len(labels))
    values[:] = np.nan
    values = pd.Index(values)
if mask.any():
    values = values.get_values()
    values[mask] = np.nan
    values = pd.Index(values)
values.name = self.names[num]
return values
```


Labels:

Comments:
Yes, your original answer is simplest (especially because it lets numpy handle all the casting) - it's a nice combination of clever and clear compared to what I was thinking. :)  There's a pathological case where this would be far less efficient (MI initialized with many many labels, then sliced to only have a subset) because the append creates a copy, but I think your solution is better because it's less error-prone.  Thanks for taking the time to work through this example!

Please make sure you're testing this with labels that are string, date-like, integer and heterogeneous, as well as levels > 0.

@jreback should this fill with `NaT` if it's datelike?

Separate question at those who might know (@jreback @y-p et al), should MI verify that levels don't include nan and then try to consolidate downwards if they do or is it fine to just not care about that?

yep date like should be NaT

NaT is prob not missing considered missing because it's just using a view of i8 so it is preserved - so it 'works' correctly 

The problem is that, with the append method, it ends up forcing to dtype object if you append nan, so you need to check. Does that already exist? maybe in common there's something like `is_datelike` that wraps up what you need to check for that?

Given the current behavoir of `factorize()`, which does not treat NaT as a missing value, we are never going to append a nan where a NaT shoud go, so no need to care. But if that behavoir is actually a bug then we have to take care of the index type and choose the appropiate NA value.

OTOH, it is worth testing `get_level_values()` with date-like levels and NaT, but I do not think it belongs to this PR since it is working as expected currently.

On second thought, using a private method returning the correct NA value for an Index (as proposed before by @jreback) makes sense anyway. This decouples `get_level_values()` from `factorize()` which is good and then we do not have to care about which values are removed by `factorize()` and which are not, instead we are ready to reconstruct the NA values whenever they have been removed. It also provides a rationale for testing NaT here.

@jreback would take_1d in common be useful for this?

yep

`com.take_nd(arr, indexer, axis=0, fill_value=fill_value)`

you can set fill_value from a method (which DatetimeIndex) can override, something like `_fill_value`

```
def _fill_value(self):
   return np.nan
```

in DatetimeIndex

```
def _fill_value(self):
     return pd.NaT
```

I used `_na_value()` instead of `_fill_value()` because fill_value is used with a different meaning (quite the opposite actually) in other contexts. I hope it's OK.

@jtratner this looks fine to me...any more comments?

@jtratner @cpcloud anything else?

@goyodiaz need a rebase 

@jtratner go ahead and merge