Repository: pandas-dev/pandas
Pull request: BUG: Don't cast categorical nan to int
- [x] closes #28406
- [x] passes `black pandas`
- [x] tests added / passed
- [x] whatsnew entry

This raises an error when attempting to cast a `Categorical` or `CategoricalIndex` containing `nans` to an integer dtype.  Also had to remove the casting within `get_indexer_non_unique` since this won't always be possible.

Problem statement:
Converting from categorical to int ignores NaNs
#### Code Sample, a copy-pastable example if possible
```python
In [6]: s = pd.Series([1, 0, None], dtype='category')                                                                                                                                                                                            

In [7]: s                                                                                                                                                                                                                                      
Out[7]: 
0      1
1      0
2    NaN
dtype: category
Categories (2, int64): [0, 1]

In [8]: s.astype(int)                                                                                                                                                                                                                          
Out[8]: 
0                      1
1                      0
2   -9223372036854775808  # <- this is unexpected
dtype: int64
```
#### Problem description
When converting categorical series back into Int column, it converts `NaN` to incorect integer negative value.

#### Expected Output
I would expect that `NaN` in category converts to `NaN` in `IntX`(nullable integer) or `float`.

When trying to use `d.astype('Int8')`, I get an error `dtype not understood`

#### Output of ``pd.show_versions()``
```
In [147]: pd.show_versions()                                                                                                                                                                                                                   

INSTALLED VERSIONS
------------------
commit           : None
python           : 3.7.4.final.0
python-bits      : 64
OS               : Linux
OS-release       : 5.2.13-arch1-1-ARCH
machine          : x86_64
processor        : 
byteorder        : little
LC_ALL           : None
LANG             : en_US.UTF-8
LOCALE           : en_US.UTF-8

pandas           : 0.25.1
numpy            : 1.17.2
pytz             : 2019.2
dateutil         : 2.8.0
pip              : 19.2.3
setuptools       : 41.2.0
Cython           : None
pytest           : 5.1.2
hypothesis       : None
sphinx           : None
blosc            : None
feather          : 0.4.0
xlsxwriter       : None
lxml.etree       : None
html5lib         : None
pymysql          : None
psycopg2         : None
jinja2           : None
IPython          : 7.8.0
pandas_datareader: None
bs4              : None
bottleneck       : None
fastparquet      : None
gcsfs            : None
lxml.etree       : None
matplotlib       : None
numexpr          : 2.7.0
odfpy            : None
openpyxl         : None
pandas_gbq       : None
pyarrow          : 0.14.1
pytables         : None
s3fs             : None
scipy            : None
sqlalchemy       : None
tables           : 3.5.2
xarray           : None
xlrd             : None
xlwt             : None
xlsxwriter       : None
```

Hints from issue discussion:
I would expect `s.atype(int)` to raise, the same as `s.astype(float).astype(int)`

To get a nullable integer, `s.astype("Int8")` should work, but let's leave that as a second issue (we may already have one for it, not sure).
> I would expect `s.atype(int)` to raise, the same as `s.astype(float).astype(int)`
> 
> To get a nullable integer, `s.astype("Int8")` should work, but let's leave that as a second issue (we may already have one for it, not sure).

It seems like the weird value gets introduced here: https://github.com/pandas-dev/pandas/blob/master/pandas/core/arrays/categorical.py#L523 . Would this be a `numpy` issue?

```
In [1]: import numpy as np                                                      

In [2]: arr = np.array([1.0, 0.0, np.nan])                                      

In [3]: np.array(arr, dtype=np.int64)                                            
Out[3]: array([                   1,                    0, -9223372036854775808])
```
> I would expect s.atype(int) to raise, the same as s.astype(float).astype(int)

Well, but that's a bit sad because it means that once you convert to categoricals, you cannot get back :-( .
> It seems like the weird value gets introduced here: https://github.com/pandas-dev/pandas/blob/master/pandas/core/arrays/categorical.py#L523 . Would this be a numpy issue?

Thanks for tracking it down. I think that's just NumPy's defined behavior.

We might want `astype_nansafe` there (should also test with `inf`).

>  Well, but that's a bit sad because it means that once you convert to categoricals, you cannot get back :-( .

What do you mean?
> What do you mean?

Well, I meant something like if you did `s = pd.Series([1, None]).astype(int).astype(category)` and now wanted to get back to `int` somehow. The workaround seems to be `s.astype('float').astype('Int8')` for example.

I understand it's tricky, especially when dealing with nullable integers. It's true that raising is probably better than trying to be somehow clever.
Sorry, I'm still not understanding. In your example,

```python
s = pd.Series([1, None]).astype(int)
```

raises. The conversion from float to int is what raises, not to or from categorical.
Sorry, I wrote it wrongly. I meant if you do:
```
In [3]: s = pd.Series([1, None], dtype='Int8').astype('category')
```
and now wanted to get back to numerics, or more specifically e.g. `Int8`. You have to do
```
In [6]: s.astype(float).astype('Int8')
```
while it would be super cool to do just `s.astype('Int8')`
> s.astype('Int8')

That should work just fine. As I said earlier

> To get a nullable integer, s.astype("Int8") should work, but let's leave that as a second issue (we may already have one for it, not sure).


> Thanks for tracking it down. I think that's just NumPy's defined behavior.
> 
> We might want `astype_nansafe` there (should also test with `inf`).

Seems that `astype_nansafe` expects an `ndarray` but `self` is Categorical here.  Any thoughts on the best workaround?
Hmm, OK.

It's a bit unfortunate, but I we'll need to include something like

```python
if is_integer_dtype(dtype):
    if self.isna().any():
        raise ValueError(...)
```

Commit message: Add CategoricalIndex test

Patch to review:
diff --git a/pandas/core/arrays/categorical.py b/pandas/core/arrays/categorical.py
index 5929a8d51fe43..b4bd2933abc52 100644
--- a/pandas/core/arrays/categorical.py
+++ b/pandas/core/arrays/categorical.py
@@ -520,6 +520,9 @@ def astype(self, dtype: Dtype, copy: bool = True) -> ArrayLike:
             if dtype == self.dtype:
                 return self
             return self._set_dtype(dtype)
+        if is_integer_dtype(dtype) and self.isin([np.nan, -np.inf, np.inf]).any():
+            msg = "Cannot cast to int."
+            raise ValueError(msg)
         return np.array(self, dtype=dtype, copy=copy)
 
     @cache_readonly
diff --git a/pandas/tests/extension/test_categorical.py b/pandas/tests/extension/test_categorical.py
index 0c0e8b0123c03..ef4a0f43e7b8d 100644
--- a/pandas/tests/extension/test_categorical.py
+++ b/pandas/tests/extension/test_categorical.py
@@ -19,7 +19,7 @@
 import pytest
 
 import pandas as pd
-from pandas import Categorical
+from pandas import Categorical, CategoricalIndex
 from pandas.api.types import CategoricalDtype
 from pandas.tests.extension import base
 import pandas.util.testing as tm
@@ -197,7 +197,13 @@ def test_searchsorted(self, data_for_sorting):
 
 
 class TestCasting(base.BaseCastingTests):
-    pass
+    @pytest.mark.parametrize("cls", [Categorical, CategoricalIndex])
+    @pytest.mark.parametrize("value", [np.nan, -np.inf, np.inf])
+    def test_cast_nan_to_int(self, cls, value):
+        s = cls([0, 1, value])
+
+        with pytest.raises(ValueError):
+            s.astype(int)
 
 
 class TestArithmeticOps(base.BaseArithmeticOpsTests):


Code review comments:
diff_hunk: @@ -520,6 +520,9 @@ def astype(self, dtype: Dtype, copy: bool = True) -> ArrayLike:
             if dtype == self.dtype:
                 return self
             return self._set_dtype(dtype)
+        if is_integer_dtype(dtype) and self.isin([np.nan, -np.inf, np.inf]).any():
path: pandas/core/arrays/categorical.py
text: @user1:
`np.isfinite` might be better to use here.

@author:
I'm going to revert back to using `isin` because `np.isfinite` breaks with input like `Series(["1", "2", "3"], dtype="category")`

@user2:
May also need a `is_float_dtype(self.categories.dtype)`

@author:
Wouldn't we want it to raise regardless of the dtype of the categories?  E.g., `astype(int)` should raise for this object even though the categories are integers:
```
[1, 2, NaN]
Categories (2, int64): [1, 2]
```

@author:
@user2 Do we also need `not is_extension_array_dtype(dtype)` here to rule out nullable integers?

@user2:
It looks like we follow this up with by passing `self` to `np.array`, which would lose the IntegerArray anyway.