Fix multiindex loc nan
State: closed

- [x] closes #43814 
- [x] tests added / passed
- [x] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#pre-commit) for how to run them
- [x] whatsnew entry


Labels:
name: Bug
name: Groupby
name: MultiIndex
name: Stale

Comments:
Hello @CloseChoice! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:









There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2021-12-08 20:18:23 UTC
@github-actions pre-commit
cc @phofl if you can have a look
> This is not a loc bug. The MultiIndex coming from the groupby operation is corrupt. It has the codes:
> 
> ```
> [[0, 0], [0, 1]]
> ```
> 
> This should be
> 
> ```
> [[0, 0], [0, -1]]
> ```
> 
> The nans are not identified as missing values, hence the incorrect behavior in subsequent operations. We have to fix this in the groupby. I think this is coming from the na_sentinel change in `factorize`

The line that is responsible for setting the codes to 1 is [here](https://github.com/pandas-dev/pandas/blob/master/pandas/core/algorithms.py#L781). But changing this so that we have the codes `[[0, 0], [0, -1]]` results in dropping the `[0, np.nan]` key. The reason for this is the function `pandas._libs.hashtable.Int64HashTable.get_labels_groupby` which actually returns only positive values. But fixing this wouldn't fix the problem either, that drills down to `result_index` (and inside that to `decons_obs_group_ids`). Which would set the index correctly but somehow the result of the aggregation function is still not correct. I'll look deeper into the change in `factorize` but at the moment it looks to me that there are more changes to be made than just in `factorize`

I tried another approach: I save the `np.nan` encoding in `codes` in a newly introduced state variable `_na_placeholder` and after all the transformations are done, I replace the placeholder with `-1` again, so that everything should be as expected. 

Let me know what you think about that.
@github-actions pre-commit
If this is not a regression https://github.com/pandas-dev/pandas/issues/43814#issuecomment-940920400, I would prefer to not do this for 1.3.x
Yeah agreed
@CloseChoice in the latest commits, the release note has been lost. Can you add back to 1.4 instead.
@pre-commit
this is WAY more complicated that before. Can you show show the groupby example and what you get now and what your expected is?
> this is WAY more complicated that before. Can you show show the groupby example and what you get now and what your expected is?

I totally agree that this is way more complex. But as the code is in the moment, I cannot simply exchange in [here](https://github.com/pandas-dev/pandas/blob/9b2bb732f09d8e870d806b337b97d3d99fd5b118/pandas/core/algorithms.py#L781)
```python
    if not dropna and code_is_na.any():
        # na_value is set based on the dtype of uniques, and compat set to False is
        # because we do not want na_value to be 0 for integers
        na_value = na_value_for_dtype(uniques.dtype, compat=False)
        uniques = np.append(uniques, [na_value])
        codes = np.where(code_is_na, len(uniques) - 1, codes)
```
because downstream the `-1` codes will be filtered out. The alternative would be to change the downstream code to be able to deal with `-1` codes. I have looked into that solution but that adds complexity, too. 

I guess you would like to have the explicit expected results in the tests. So would I, but actually I didn't manage to compare a `FrozenList` of `np.arrays` properly. I couldn't compare with `pd.assert_index_equal` because it doesn't get the `np.nan` values right. Therefore I'll post the explicit codes here with code to reproduce:

```python
import pandas as pd
import numpy as np

for na in [np.nan, pd.NaT, np.datetime64("NaT", "ns"), np.timedelta64("NaT", "ns")]:
    df = pd.DataFrame(
        {
            "temp_playlist": [0, 0, 0, 0],
            "objId": ["o1", na, "o1", na],
            "x": [1, 2, 3, 4],
        }
    )

    grouped_df = df.groupby(by=["temp_playlist", "objId"], dropna=False)["x"].sum()
    expected = pd.MultiIndex.from_arrays(
        [[0, 0], ["o1", na]], names=["temp_playlist", "objId"]
    )
    result = grouped_df.index
    print(f"This is na {na} with type {type(na)} and the corresponding codes {result.codes}")
```
In all these cases we want the expected codes to be `[[0, 0], [0, -1]]`.
Output on master:
```
This is na nan with type <class 'float'> and the corresponding codes [[0, 0], [0, 1]]
This is na NaT with type <class 'pandas._libs.tslibs.nattype.NaTType'> and the corresponding codes [[0, 0], [0, 1]]
This is na NaT with type <class 'numpy.datetime64'> and the corresponding codes [[0, 0], [0, 1]]
This is na NaT with type <class 'numpy.timedelta64'> and the corresponding codes [[0, 0], [0, 1]]
```
and on the fix:
```
This is na nan with type <class 'float'> and the corresponding codes [[0, 0], [0, -1]]
This is na NaT with type <class 'pandas._libs.tslibs.nattype.NaTType'> and the corresponding codes [[0, 0], [0, -1]]
This is na NaT with type <class 'numpy.datetime64'> and the corresponding codes [[0, 0], [0, -1]]
This is na NaT with type <class 'numpy.timedelta64'> and the corresponding codes [[0, 0], [0, -1]]
```


@github-actions pre-commit
#36060 is related
> #36060 is related

this PR also resolves #36060. I can add tests for that if that is wanted. Or do you suggest any other way to solve that?
yes pls
> yes pls

added the tests. Please note one thing:

```python
>>> import pandas as pd
>>> import numpy as np
>>> df = pd.DataFrame(
...     {
...         "animal": ["Falcon", "Falcon", "Parrot", "Parrot"],
...         "type": [np.nan, np.nan, np.nan, np.nan],
...         "speed": [380.0, 370.0, 24.0, 26.0],
...     }
... )
>>> speed = df.groupby(["animal", "type"], dropna=False)["speed"].first()
>>> # Reconstruct same index to allow for multiplication.
>>> ix_wing = pd.MultiIndex.from_tuples(
...     [("Falcon", np.nan), ("Parrot", np.nan)], names=["animal", "type"]
... )
>>> wing = pd.Series([42, 44], index=ix_wing)
>>> 
>>> result = wing * speed
>>> wing.index.dtypes
animal    object
type      object
dtype: object
>>> speed.index.dtypes
animal     object
type      float64
dtype: object

# dtype of multiindex is dependent of the order of factors
>>> (wing * speed).index.dtypes
animal    object
type      object
dtype: object

>>> (speed * wing).index.dtypes
animal     object
type      float64
dtype: object
```

Therefore I only multiply `wing * speed` and not `speed * wing`. I guess this is a bug.
@github-actions pre-commit
@github-actions pre-commit
This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.
Thanks for the PR, but it appears to have gone stale. If interested in continuing please merge the main branch and move the whatsnew note to 1.5, and we can reopen.