BUG: Ensure 'coerce' actually coerces datatypes
State: closed

Changes behavior of convert objects so that passing 'coerce' will
ensure that data of the correct type is returned, even if all
values are null-types (NaN or NaT).

closes #9589


Labels:
name: Bug
name: Dtype Conversions
description: Unexpected or buggy dtype conversions
name: API Design

Comments:
Definitely not ready, but close enough to begin discussing the changes in behavior I am proposing.

Essentially I am suggesting that if one of the flags is 'coerce' that the other must not be 'coerce' and that when 'coerce' is used then the returned array will always have the target type.

If other combinations of the keyword arguments are set to `True` (and no `'coerce'`), then the behavior is the same.  

The big change is that if `'coerce'` is passed, then no other attempts are made to convert to other types in the event that all converted values are null-types.

pls rebase on master and repush had an issue with some builds

I had addressed all the points raised, and have two issues remaining.
1. I am using `asanyarray` since the returned type is TimedeltaIndex which is not what is expected from this function, and causes errors in other places it is called since it doesn't have all methods of an `ndarray`.  Maybe I shoudl just use `np.array`
2. The behavior in the current version is different for `convert_dates`/`convert_timestamps` and `convert_numeric`.  `c_d` and `c_ts` both will do nothing if given a mixed series -- that is, one which has some datetimes/timestamps but some non-convertibles.  `convert_numeric` will return `nan` for non-convertibles and numbers when convertible.  I am proposing to alter the behavior so that `True` will always return object-dtype when faxing mixed arrays.  Is this reasonable?

1. pass `box=False` to `to_timedelta` rather than coercing (as this will simply not box it into a `TimedeltaIndex`).
2. can you give an example (current and proposed?)

The flags are dates, numeric, timestamps.  When it is `True, False, False` or `False,False,True` then the returned series is essentially not converted since many of the objects can't be converted to datetimes or timestamps.

However, when it is `False,True,False` then the returned type is `float64` with `NaN`s for the not-convertible types.

``` python
import pandas as pd
import datetime as dt
dt = dt.datetime.now()
td = dt - dt
s = pd.Series([1.0,'1.0',dt,td])

s
Out[6]: 
0                             1
1                           1.0
2    2015-06-04 10:56:04.421566
3                       0:00:00
dtype: object

s.convert_objects(True,False,False)
Out[7]: 
0                             1
1                           1.0
2    2015-06-04 10:56:04.421566
3                       0:00:00
dtype: object

s.convert_objects(False,True,False)
Out[8]: 
0     1
1     1
2   NaN
3   NaN
dtype: float64

s.convert_objects(False,False,True)
Out[9]: 
0                             1
1                           1.0
2    2015-06-04 10:56:04.421566
3                       0:00:00
dtype: object
```

Proposed correct output would not be converted -- not even the string '1.0'. **Not sure if the string should be converted or not here**

``` python
s.convert_objects(False,True, False)
Out[9]: 
0                           1
1                           1.0
2    2015-06-04 10:56:04.421566
3                       0:00:00
dtype: object
```

The reason this happens in 0.16.x is that the conversion is made wiht 

``` python
new_values = lib.maybe_convert_numeric(values, set()), coerce_numeric=True)
```

in all cases, so that `coerce_numeric=True` is always passed.  

Some further thoughts:

The `'coerce'` options are fine -- that it `'coerce'` will always return the expected type irrespective of what is given.

The `True`/`False` options should have the following behavior:
1. For dates or timestamps, only convert if all objects are dates/timestamps
2. For numeric types, convert if _anything_ is numeric-like -- either a number or a string convertible to a number.

This means that for numeric types, the only difference between `'coerce'` and `True` is when all values are `NaN` - in which case `True` will return the original array while `'coerce'` will return an array with all `NaN`s.

This still leaves the functions having a bit of _magic_, but I think being stricter would create a lot of headaches.

It should be close now.  I had to fix an issue in `groupby` so this needs some checking.  Still need to document things.

@bashtage looks pretty good.

btw, I think now might be a good time to change the actual argument names as well. ( and deprecate the originals).

so instead of:

`convert_objects(self, convert_dates=True, convert_numeric=False, convert_timedeltas=True, copy=True)`

maybe?
`convert_objects(self, datetime=True, numeric=False, timedelta=True, copy=True)`

 or _maybe_ we just add to directly to `.astype(....)` instead? (in effect make the `dtype` arg optional if the others are passed)

further I would not object to additional args `floating=False,integer=False` (to be cooperative with `numeric`, and would clearly have to have a precedence order), unless that is MORE confusing.

I suppose my ideal API would look like

`convert_objects(datetime=True, numeric=False, timedelta=False, coerce=False, copy=True)`

so that the values for the keywords would always be `bool` and coercions would happen if `coerce` was set.

The behavior would be as follows:
1. If `coerce=True` then at most one of `datetime`, `numeric` or `timedelta` must be `True`, and the returned type will always have the correct type with all non-convertable values set to null-types.
2. If `coerce=False`, then soft conversions will be attempted where a conversion is successful if **all** values can be converted to a particular type.  If any values are not-convertible, then the original object array is returned.

The current version satisfies 1 but not 2 since the behavior of `convert_numeric=True` is to always convert if at least one value is convertible to numeric, while `convert_dates` and `convert_timedelta` will only convert if all are convertible.

I'm also not so sure about the `copy` flag since any conversion will be changing the underlying blocks since a columns will be changing from object to another type.  What would `copy=False` be expected to do?  Replace "inplace"?  Return the same object with different blocks?

`copy=True` is the default and in reality should always be `True`. Only in very very specific circumstances (and internally only) do I support `copy=False`. Its not really 'public' in that sense, so maybe should be `_copy=True` to mark it as an internal option (or could just dispense with flag, and make another func for internal consumption).

LMK what you think of this version of the API.  It is not

``` python
df.convert_objects(datetime=True, numeric=False, timedelta=True, coerce=False, copy=True)
```

and if `coerce=True`, then only one of the three options can be `True`.  Right now using `coerce` is a little tedious since it require setting one of `datetime` or `timedelta` to be `False`.  Have though about making `timedelta=False` as a default.

@jreback It is green now so the major question is whether the API change is reasonable or not?  Will hold off on doc changes until then.

@bashtage I am ok with the API change (small typo on the `timedeltas` should be `timedelta`). This will have to be in 0.17.0 as we will deprecate the old. (0.16.2 coming later this week).

@jreback Should the `@deprecated` be in a separate PR so it can be merged before this one?

@bashtage no, in this PR. It doesn't make sense to deprecate something w/o replacing it. (unless its complete vestige), in this case you are renaming things, the canonical reason to use deprecate

@jreback I think this is ready when you are.

Some other questions:
1. Now, the `convert_numeric` set to True or 'coerce' does not seem to differ (it always coerces?)
   
   ```
   In [30]: s = Series(['a', 'b', '1'])
   
   In [31]: s.convert_objects(convert_numeric=True)
   Out[31]:
   0   NaN
   1   NaN
   2     1
   dtype: float64
   
   In [32]: s.convert_objects(convert_numeric='coerce')
   Out[32]:
   0   NaN
   1   NaN
   2     1
   dtype: float64
   ```
   
   Is this still the same? (I assume so from the tests) If so, this should maybe be more clear from the docs?
2. The fact that you now have to do `df.convert_objects(datetime=False, numeric=True, coerce=True)` (as only one of the datetime, numeric or timedelta can be True when coerce=True). Is there nothing we can do about this? (because it is rather ugly?) 
   Maybe we could set `datetime=False` as the default? (we can detect this case if all three are False, and then use `datetime=True` (and possibly raise a warning))
3. Currenly, when having no convertible values, that original is always returned, also when specifying 'coerce':
   
   ```
   In [26]: s = Series(['a', 'b', 'c'])
   
   In [27]: s.convert_objects(convert_numeric=True)
   Out[27]:
   0    a
   1    b
   2    c
   dtype: object
   
   In [28]: s.convert_objects(convert_numeric='coerce')
   Out[28]:
   0    a
   1    b
   2    c
   dtype: object
   ```
   
   With this PR, from the tests it seems that this now gives all NaNs? I think this is a reasonable change, but this should then also be listed in the API changes in whatsnew? 
   EDIT: Ah, I see this was the original issue .. :-) But it should then be mentioned in the whatsnew as well I think

@jorisvandenbossche `.convert_objects` generally is not used very much as almost all coercion is already done automatically, or via explicit converters like `to_datetime/to_timedelta`. Aside from some internal use, the main use is the `to_numeric` which is basically an `.astype` with that handles non-convertibles.

I suppose another option is to make `.convert_objects` private, and add `pd.to_numeric` to keep things clear; a third option is to add a `coerce` argument to `.astype` to allow have a nicer API.

Lastly, I suppose there is a need for a function to just 'take this object series and type it correctly'. Hence having all of these in one function. I don't think this actually a common case, but at times I suppose could be useful.

@jreback Although maybe not that much, but it _is_ used (for example, StackOverflow gives quite some results if you search for that) 
And for the case you have (for whatever reason) mixed numeric and strings, this is a useful function no? (more flexible as `astype`)

I agree that eg adding `coerce` to `astype` would be a useful enhancement, but nonetheless, we should take care at `convert_objects` as well. And even it is regarded as internal, it should be clear what it does, and so my questions above should be answered :-)

@bashtage BTW, you have a failing test because "3.1415" seems to be parsed as a timedelta. With master:

```
In [5]: pd.to_timedelta('3.1415')
Out[5]: Timedelta('0 days 00:00:00.141500')
```

which seems wrong in any case (@jreback maybe related to the changes there #10396 ?)

@jorisvandenbossche you are looking at old posts on SO, this WAS used quite a but should not be necessary in quite a few versions, except in a small number of cases.

Since we have a proposed API change (to make a better naming convention), though which is back-compat. I think it might be nicer to simply move this to `.astype` as that is the use case. 

would be also ok with renaming with to `.convert`

`Timedelta('3.1415')` is wrong in 0.16.2 and master. So will open a separate issue for this. Was ok with accepting single integers e.g. `Timedelta('3')` which is 3ns. But now that I think about it these should raise (I don't reallly remember why I originally did that). They don't have units in this case (different story when you are explicity passing to `to_timedelta`). So its ambiguous.

@jorisvandenbossche This failure is my reward for religously rebasing :-/

## API Design

### The reason for `convert_objects`

Inside pandas, convert objects is frequently used to re-convert columns back to datetime after some aggregation operation, for example a `grouby`.  

This is why the proposed API is so ugly: I wanted to minimize internal refactoring of `convert_objects` and having defaults `datetime=True` with everything else `False` achieved this goal.

Externally the main reason to use `convert_objects` **without** `coerce=True` is to magically convert a DataFrame of string values to one of the three supported formats.  The logic is as follows:

```
for col in data:
    if all of data[col] is convertable to datetime:
        convert data[col]) to datetime
    elif any of data[col] is convertable to numeric
        convert data[col] to numeric (int, float as appropriate)
   elif all of data[col] is convertible to timedelta:
       convert data([col]) to timedelta
```

This is strange since the behavior of partially convertable columns is different depending on the type.  This is how is is in 0.16 and I didn't want to alter this behavior since this magic is used in various places.

### What should the API look like

For single type conversions, `Series` and `DataFrame` should use `astype(*,coerce=True)` which would do exactly as told - no magic, no guessing, just 100% consistent.

This would mean `convert_objects` could have the `coerce` keyword **removed** and so would only be for soft conversions.

> Now, the convert_numeric set to True or 'coerce' does not seem to differ (it always coerces?)

I think this is correct - if the column is all strings not parsable to numbers, this will return the original data irrespective of `coerce`.  More-or-less the logic of `convert_objects` in 0.16 is deeply flawed (there are some errors in the code).  For example

https://github.com/pydata/pandas/pull/10265/files#diff-21cb6655a9828d526610249e3d6cb16eL1921

cannot do anything since new_values isn't even set.  

Coverage of this internal function is not great so no wonder this slipped through.

> The fact that you now have to do df.convert_objects(datetime=False, numeric=True, coerce=True) (as only one of the datetime, numeric or timedelta can be True when coerce=True). Is there nothing we can do about this? (because it is rather ugly?)
> Maybe we could set datetime=False as the default? (we can detect this case if all three are False, and then use datetime=True (and possibly raise a warning))

I am hesitant to do this since there are lots of internal places it is assumed `convert_dates=True` in 0.16.

ok, then let's deprecate `raise_on_error` in `.astype` and replace it with `coerce` (you may need to slightly adjust the logic in `core/internals/_astype`).

Then `convert_objects` is much simpler (only does soft conversions).

Ok, will work on this in the next few days

> The fact that you now have to do df.convert_objects(datetime=False, numeric=True, coerce=True) (as only one of the datetime, numeric or timedelta can be True when coerce=True). Is there nothing we can do about this? (because it is rather ugly?)
> Maybe we could set datetime=False as the default? (we can detect this case if all three are False, and then use datetime=True (and possibly raise a warning))
> 
> > I am hesitant to do this since there are lots of internal places it is assumed convert_dates=True in 0.16.

This is simply not possible, nor desired. `.convert_objects` is NOT meant for public consumption except in a very special case (naming safe-astyping). If this is moved to `.astype` then no reason to have this public anymore. 

So should deprecate.

Having started looking at `astype` I'm less sure about what I shoudl expect to happen.

For example, what should happen if I have 

```
df = pd.DataFrame({'a':[1.3,3.1415]})
df.astype(np.int, coerce=True)
```

Converting object dtype to other things makes sense, but `coerce` doesn't seem to be generally applicable without lots of arbitrary choices (like how to change floats to integers).

I suppose the big advantage of `convert_objects` is that it is only for object columns, and not for anything else.  

Maybe `astype(*,coerce_object=True)` as a special case, or just `coerce` but with a note/warning that `coerce` is only applicable when when the input `dtype` is `object`?

My previous example isn't useful.  This is maybe clearer:

```
df = pd.DataFrame({'a':['2015-01-01','2015-01-02']}).astype(np.datetime64)
df.astype(np.float64, coerce=True)
```

or similarly

```
df = pd.DataFrame({'a':[1.0,np.nan]})
 df.astype(np.int64, coerce=True)
```

What would be expected?  It seems a general dtype to dtype conversion with coercion might be very difficult and complex.