BUG: Fix issue with old-style usage in convert_objects
State: closed

Fix to temporary allow passing 'coerce' to variables

closes #10601


Labels:
name: Bug
name: Deprecate
description: Functionality to remove in pandas

Comments:
lgtm

I don't think this handles the case where you did two different types at the same time? Eg `df.convert_objects(convert_numeric=True, convert_dates='coerce')` ?

That case was wrong in 0.16 so I don't think it should be fixed.  Coerce should mean one and only one type allowed. 

The only question is whether I should be setting the non-coerce string to False, or just pass them through.

A different strategy would be to set any input that is coerce to `True` and ensure `coerce=True` and then let the error raise.

An example with simultaneous numeric and dates, which already worked in 0.15. Was it a bug that it worked?

```
In [1]: df = pd.DataFrame({'a':[1.0, '2.3'], 'b':['1', "4"], 'c':['a', pd.Timestamp('2012-01-01') ]})

In [2]: df.dtypes
Out[2]:
a    object
b    object
c    object
dtype: object

In [3]: df.convert_objects(convert_numeric=True, convert_dates='coerce')
Out[3]:
     a  b          c
0  1.0  1        NaT
1  2.3  4 2012-01-01

In [4]: df.convert_objects(convert_numeric=True, convert_dates='coerce').dtypes
Out[4]:
a           float64
b             int64
c    datetime64[ns]
dtype: object

In [5]: pd.__version__
Out[5]: '0.15.2'
```

``` python
import datetime as dt
df = pd.DataFrame({'a':['1.','2010-01-01',dt.datetime.now()-dt.datetime.now()]})
```

Any of these either don't work or don't make sense:

```
df.convert_objects(convert_dates='coerce', convert_numeric='coerce',convert_timedeltas=True)
df.convert_objects(convert_numeric=True,convert_timedelta='coerce')
df.convert_objects(convert_dates=True, convert_numeric=True,convert_timedeltas=True)
```

The issue is there are was a a very specific order as to what happens, and once one has satisied some criteria, then the others weren't attempted.  For example, in the final one, why is the result numeric?

it only did the conversions (even soft conversions) if it had an `object` dtype. So all were `True` (which is weird, but I suppose possible), then it would attemp them in order : datetime, numeric, timedelta. numeric would almost always succeed before timedelta (which is only a soft conversion anyhow).

You will warn if any of the above examples are passed, so that is ok!

@bashtage @jreback coming back to this: 
- Ah, I see that now it is disallowed to do multiple conversions when `coerce=True`. But, nonetheless, it _did_ work previously (with eg `df.convert_objects(convert_numeric=True, convert_dates='coerce')`, see my example above).  
  I was thinking: would it be better to just pass this through, so you get an error instead of a warning?  
  it seems reasonable to me to ensure that if someone did this before with the old style, this keeps working (his/her code does not break, the example I give above does not work now on master, and also not yet with this PR I think), but this is not possible with the new logic?
- Maybe we should also add a note in the docstring of `convert_objects` that if multiple conversions are done at the same time, this happens with a particular order? And also, that this is not allowed when using `coerce=True` (you get an error for that, but it is not mentioned in the docstring)

Further, the default was previously to do date conversions. Now it is stated in the docs that the default is changed to do nothing. However, if I try on master, it seems that by default it does do numeric conversions:

```
In [29]: pd.__version__
Out[29]: '0.16.2+220.ga677217'

In [30]: df = pd.DataFrame({'a':[1.0, '2.3'], 'b':['1', "4"], 'c':['a', pd.Times
tamp('2012-01-01') ]})

In [31]: df.dtypes
Out[31]:
a    object
b    object
c    object
dtype: object

In [32]: df.convert_objects()
Out[32]:
     a  b                    c
0  1.0  1                    a
1  2.3  4  2012-01-01 00:00:00

In [33]: df.convert_objects().dtypes
Out[33]:
a    float64
b      int64
c     object
dtype: object

In [34]: df.convert_objects(numeric=False).dtypes
Out[34]:
a    float64
b      int64
c     object
dtype: object
```

So it seems that the `numeric` keyword has no effect.

@jorisvandenbossche The purpose of `.convert_objects` is conversion. whether it be `soft` or `force`.  Yes it defaulted to convert datetimes (but only in a trivial case when it was `object` dtyped but was actually an array of `Timestamps`). So calling it will do _some_ conversions (like above its a soft numeric conversion).

So this should not generally be necessary from a callers perspective for datetimes/timedeltas as we already have a well-defined way of doing this (e.g. `to_datetime/to_timedelta`). (though it IS used internally). 

How about adding a `to_numeric` analagously? and making `.convert_objects` private. These are _kind_ of like `.astype` but more general.

Downside is you cannot then say to pandas "just figure this out", by doing `.convert_objects(datetime=True, timedelta=True, numeric=True)`. (which IMHO is kind of dumb anyhow though).

@bashtage can you update and respond to above comments.

What was the decision?  Make `convert_objects` private and add a `to_numeric`, so that there would be a trio of primitive coercers? I suppoe if this is the way forward I would restore `convert_objects` to its previous version (and fix the big bug in it) and then start its deprecation. 

I think it would be ok to add `to_numeric` as we already have `to_datetime/to_timedelta`. We could leave `.convert_objects` as is (even though you are slightly changing the defaults). I don't think that is a big deal. What is the bug?

The original `covert_objects` has a path (with datetime conversion) that doesn't return the right thing.  Something like 

``` python
new_values = some_fn(x)
# A step like this is missing
if not np.isnan(new_values).all():
    values = new_values
# End missing
return values
```

This isn't in master since my modified `convert_objects` is quite a bit different.

ok so their was an old bug which you fixed, but what are we missing with the new one that is necessary for back-compat? can you give an exmple

> ok so their was an old bug which you fixed, but what are we missing with the new one that is necessary for back-compat? can you give an exmple

The version currently in master has a different signature.  In particular, `coerce` is now a `bool`.  Previously coercing was done using arguments like `convert_numeric='coerce'`.  This was done mostly to make it clear that one can only coerce one type.   For example, you can run

```
df = pd.DataFrame([pd.NaT, '1.', dt.datetime(2015,1,1)]
df.convert_objects(convert_dates='coerce', convert_numeric='coerce'
```

which always struck me an horribly ambiguous as to what you get - the returned type depends on the order of the code in the function, which seems like a fragile ("magic") design.

This can't happen with the new one since 

```
df = pd.DataFrame([pd.NaT, '1.', dt.datetime(2015,1,1)]
df.convert_objects(datetimes=True, numeric=True, coerce=True)
```

will raise since this is ambiguous.

I suppose there are a few different options:
1. Restore to the 0.16.2 version, plus fix the bug
2. Option 1 plus check for ambiguous calls
3. Option 2 plus keep new keyword argument names
4. Go forward with a fundamentally different signature, which is in master

There is one final issue with `covert_objects`, which is what started me down this path, which is what to do about `convert_numeric='coerce'` which does not actually coerce if there are no numeric types.

@bashtage I think that you have preserved back compat enough. I would add a warning box in the whatsnew and the docs saying if you are relying upon the 'magic' then you need to be forewarned.

Separately I think we should go forward with `to_numeric`, can you make an issue? (We can just leave `.convert_objects` or deprecate the entire method in the future then).

I have a PR nearly ready for `to_numeric` should have that soon.

I'm all for not changing things, but I was under the impression that @jorisvandenbossche felt strongly about supporting the same arguments.

If I am reading the above correctly, then the objection is that the following _did_ work.

`df.convert_objects(convert_numeric=True, convert_dates='coerce')`

It seems to me that this is easily cause though and might as well raise on this. (e.g. you are seeing the deprecated args, you know that the user is trying to do).

It _may_ be that intercepting this via the decorator is a bit tricky though (as you are using the mapping; maybe preserving the args and then just warning as you see them is better)?

@jorisvandenbossche comments? this lgtm

@jorisvandenbossche ?

merging. I think this is fine.