Astype keeps nan when converting into string
State: closed

- closes #25353 
- 1 test added
- passes black pandas
- whatsnew entry



Labels:
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate

Comments:
So just some thoughts - I wouldn't consider the astype -> "nan" a feature as much as a bug, so while potentially breaking I somewhat feel like a deprecation cycle for buggy behavior is overkill (though others may differ)

w.r.t. the skipna argument I don't see why users would ever really want "nan" when `np.nan` is available so maybe not needed?
Right, the skipna argument would only be necessary if we're deprecating.

On Tue, Aug 27, 2019 at 10:54 AM William Ayd <notifications@github.com>
wrote:

> So just some thoughts - I wouldn't consider the astype -> "nan" a feature
> as much as a bug, so while potentially breaking I somewhat feel like a
> deprecation cycle for buggy behavior is overkill (though others may differ)
>
> w.r.t. the skipna argument I don't see why users would ever really want
> "nan" when np.nan is available so maybe not needed?
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/28176?email_source=notifications&email_token=AAKAOIVGVMT73RPUWCAU2JLQGVEZXA5CNFSM4IQFI3MKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5IHBYA#issuecomment-525365472>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AAKAOIVVNTOGMBZSQQBDPMLQGVEZXANCNFSM4IQFI3MA>
> .
>

> I wouldn't consider the astype -> "nan" a feature as much as a bug, so while potentially breaking I somewhat feel like a deprecation cycle for buggy behavior is overkill 

I'm +1 on having a deprecation cycle.  I agree that this behavior feels buggy but it's actually consistent with numpy's behavior, and silently breaking consistency with numpy could be surprising to users.

For example, using `astype(str)` on an `object` dtype array converts `np.nan` -->`'nan'`:
```python
In [1]: import numpy as np; np.__version__
Out[1]: '1.16.4'

In [2]: a = np.array(['foo', 1, np.nan, 3.14], dtype=object)

In [3]: a
Out[3]: array(['foo', 1, nan, 3.14], dtype=object)

In [4]: a.astype(str)
Out[4]: array(['foo', '1', 'nan', '3.14'], dtype='<U4')
```
Also note that when constructing `a` as above, if `dtype=object` isn't explicitly specified the  string conversion will silently occur:
```python
In [5]: np.array(['foo', 1, np.nan, 3.14])
Out[5]: array(['foo', '1', 'nan', '3.14'], dtype='<U4')
```
Just so you're all aware, this non-feature also appears to impact NoneType:

```python
>>> na = np.array(['foo','bar',5,np.nan,None])
>>> [type(x) for x in na]
[<class 'str'>, <class 'str'>, <class 'int'>, <class 'float'>, <class 'NoneType'>]

>>> da = pd.Series(na)
>>> [type(x) for x in da.astype('str')]
[<class 'str'>, <class 'str'>, <class 'str'>, <class 'str'>, <class 'str'>]
>>> [type(x) for x in da.astype('str',skipna=True)]
[<class 'str'>, <class 'str'>, <class 'str'>, <class 'str'>, <class 'str'>]
```

Loosely thought about this after reading through #27949 but do we care to differentiate between the following like numpy?

```python
>>> np.array([1, np.nan, 3]).astype(object)
array([1.0, nan, 3.0], dtype=object)

>>> np.array([1, np.nan, 3]).astype(str)
array(['1.0', 'nan', '3.0'], dtype='<U32')
```

Coercing to an object in NumPy would maintain the actual NA value, but to a string would write as `'nan'`. I suppose this makes more sense in that world where object and string types are different, but I'm just wondering what kind of churn we would be causing if we actually did want `astype(str)` to stringify `nan` with a proper `StringArray` in place
I think that the `np.nan -> 'nan'` behavior is surprising to most people the first time they see it. So by default, I would want to exclude NA values from being astyped (after a deprecation cycle).
Hello @makbigc! 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 2020-01-04 06:25:32 UTC
Linting test failed (i.e., black formatting). But I can pass the black formatting check in my own machine. Where did I do wrong?
@makbigc might want to give a pre-commit hook a try:

https://pandas.pydata.org/pandas-docs/stable/development/contributing.html#python-pep8-black

In any case can you merge master and re-push?
@jreback Anything else? Please tell me.
can you merge master and will look again
The `-W error` is set for pytest command in azure-37-numpydev.

https://github.com/pandas-dev/pandas/blob/8806ed7120fed863b3cd7d3d5f377ec4c81739d0/ci/azure/posix.yml#L55-L59

A lot of tests were failed by the FutureWarning. What should I do?
@makbigc I think need to address https://github.com/pandas-dev/pandas/pull/28176#pullrequestreview-328622327 first; the problem now is that the warning is getting thrown all over the place because `skipna` was added to `astype`

If you can adjust design per comment can re-evaluate issues from there
@WillAyd Thanks for your reply. The skipna parameter has been removed. The skipna is set True by default in `astype_nansafe`.

The tests are failed by the FutureWarning. https://github.com/pandas-dev/pandas/pull/28176#issuecomment-570247305
@jreback The tests were failed by the issue of FutureWarning. https://github.com/pandas-dev/pandas/pull/28176#issuecomment-570247305

Any resolution?
@jreback Any feedback? Thanks.
It seems this didn't make it into pandas 1.0 but also `skipna` has been removed from `astype`. What is the intended way of casting to string with NaNs in pandas 1.0?
If we are going to deprecate this, I think we need a keyword to opt in to the future behaviour, so you have the possibility to silence the warning. 

And if we are adding a keyword, it seems to make sense to use `skipna` for that, as that could (at least in 0.25) already be used for this purpose (although undocumented)
Where are we at here? Seems like the preference is for

```
>>> s = pd.Series([1, np.nan])
>>> type(s.astype(str)[1])
UserWarning("Converting NaN to string 'nan'. In the future NaN will remain as NaN. Specify skipna=True to silence this warning and adopt the new behavior, or skipna=False to restore the old behavior").
str

>>> type(s.astype(str, skipna=False)[1])  # no warning
str

>>> type(s.astype(str, skipna=True)[1])  # no warning
float
```

Is that right? If so, @makbigc are you interested in implementing that?
some time has passed since this PR was opened and pandas 1.0 has since been released.

> If we are going to deprecate this, I think we need a keyword to opt in to the future behaviour, so you have the possibility to silence the warning.
> 
> And if we are adding a keyword, it seems to make sense to use `skipna` for that, as that could (at least in 0.25) already be used for this purpose (although undocumented)

So, for me, a deprecation cycle on old (incorrect) behaviour and keep the skipna keyword (but deprecate it) is probably now a requirement to keep a stable api through to pandas 2.0.

This approach addresses both the regression in #31708 and the unexpected behaviour of  #25353


@makbigc can you merge master to resolve conflicts, fix-up failing tests and address https://github.com/pandas-dev/pandas/pull/28176#issuecomment-595887420
@makbigc closing as stale. ping if you want to continue.
Hello, @simonjayhawkins, @TomAugspurger 

I would like to continue this PR as it seems it would solve multiple issues, I just want to declare everything that must be done before and to ensure that this new behaviour is still preferred.

- [ ]  Add _skipna_  keyword default to False at `pandas/core/generic.NDFrame.astype` method. Following the UserWarning https://github.com/pandas-dev/pandas/pull/28176#issuecomment-595887420
- [ ] Document this new behavior.
- [ ] Start deprecation cycle

Open Issues related
- #25353 
- #28454
- #31708

Unmerged PRs related
- #35060