ENH: add downcast to pd.to_numeric
State: closed

Title is self-explanatory.  Closes #13352.


Labels:
name: API Design
name: Numeric Operations
description: Arithmetic, Comparison, and Logical operations

Comments:
## [Current coverage](https://codecov.io/gh/pydata/pandas/pull/13425?src=pr) is **84.31%**

> Merging [#13425](https://codecov.io/gh/pydata/pandas/pull/13425?src=pr) into [master](https://codecov.io/gh/pydata/pandas/branch/master?src=pr) will increase coverage by **<.01%**

``` diff
@@             master     #13425   diff @@
==========================================
  Files           138        138          
  Lines         51157      51176    +19   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          43131      43151    +20   
+ Misses         8026       8025     -1   
  Partials          0          0          
```

> Powered by [Codecov](https://codecov.io?src=pr). Last updated by [5701c69...4758dcc](https://codecov.io/gh/pydata/pandas/compare/5701c69369264f3aa6f571384602ceec1133dabc...4758dccce15b60b4ccb5a2c2ca06250af6e55c0d?src=pr)

@jreback : Refactored `na_values` and added the version tags.  Travis is still happy, so ready to merge if there are no other concerns.

lgtm. @jorisvandenbossche 

small issue is whethere the kwargs are the right names; further `use_unsigned` deps on `compat_ints` which is a little awkward.

maybe:

`compat_ints`, `compat_ints_to_unsigned` ? (and decouple them). I know its longer, but its more explicit.

@jreback : I see what you're saying, but the decoupling would raise the issue of which one takes precedence?  i.e. what happens if for some reason or another, someone passes in `compact_ints=1` and `compact_ints_to_unsigned=1`?  Also, not sure what I would necessarily say that `use_unsigned` totally depends on `compact_ints` - the way I look at it is that it further parameterizes the compacting.

@gfyoung that would be an error, both options cannot be True.

@jreback : hmmm, on the fence regarding this - will wait to see what @jorisvandenbossche says

At the moment, the implementation in this PR is not really a substitute for the `pd.read_csv(... compact_ints=True)` I think? As it will only do something when there is actually a conversion from object dtype to integer dtype.

Of course it could be easily changed that the keyword is also used when the dtype is already numeric (the `if com.is_numeric_dtype(values): pass` at the moment). 
But, after second thought, I am actually not sure if `to_numeric` is the appropriate place for this functionality IMO. As then you would feed integer data to `to_numeric`, not to "convert it to numeric dtype" (the current purpose of the function), but to downcast them (which broadens the scope of the function). And it feels this should actually belong in a separate function.

@jorisvandenbossche I view `to_numeric` as basically `.astype`, with better error handling. So in that vein, I don't think it has to be `object` to begin with, though maybe we should _also_ add a `dtype=` option.

alternatively, _maybe_ add these instead to `.astype`.

and @gfyoung certainly remove the non int64-sentinels though.

@jorisvandenbossche : that is not correct - downcasting occurs so long as the **converted data** is of integer dtype, **irregardless** of its starting dtype i.e. data that is already integral can also be downcasted.  This is true in `read_csv` and in `to_numeric` as well (with my changes).

While I do see what you are saying that this change could be viewed as expanding the functionality beyond what it was originally intended for, `compact_ints` and `use_unsigned` can be viewed as further specifying the manner in which data is casted to numerical values.  From this viewpoint, this added functionality remains within the bounds of what this function was originally designed to do.

@jreback : fair enough regarding sentinels - however, as they are used in `pd.read_csv`, I added another optional parameter to `downcast_int64` to check for them by default (for `read_csv` purposes) and then passed in `False` explicitly for `to_numeric`.

@jreback @jorisvandenbossche : Removed the sentinels behavior in `to_numeric` with the addition of the `use_sentinels` parameter in `downcast_int64` (okay since this is internal), and Travis is happy still.

@gfyoung yeah we need to remove the sentinel behavior, but that needs to wait for 0.18.2

Well at the very least it is not in `to_numeric` anymore.  But fair enough, it can also be removed from `read_csv` in 0.19.0 (which I presume is what you actually meant), as I also do not understand their purpose or what use they would have now.

The changes to `to_numeric` are completely separate from `read_csv` (current behaviour is unaffected), so if there are no other concerns, this should be good to mere then.

@jreback : removed the stray "sentinel" and Travis is still happy.  Ready to merge if there are no other concerns.

> @jorisvandenbossche : that is not correct - downcasting occurs so long as the converted data is of integer dtype, irregardless of its starting dtype

What I meant is:

```
In [2]: pd.to_numeric(pd.Series([1,2,3]), compact_ints=True)
Out[2]:
0    1
1    2
2    3
dtype: int64
```

Which should also work I think?

@jorisvandenbossche : Ah and argh!  You're right, that's a bug.  Should be fixed now.

@jreback @jorisvandenbossche : Squashed the failure to compact integers bug when the data was already integer.  Travis is happy with the changes and the added tests.  Ready to merge if there are no other concerns.

You get a rather cryptic error when passing integers that are not int64:

```
In [4]: pd.to_numeric(pd.Series([1,2,3], dtype='int32'), compact_ints=True)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-4-ffd8eb522bf1> in <module>()
----> 1 pd.to_numeric(pd.Series([1,2,3], dtype='int32'), compact_ints=True)

c:\users\vdbosscj\scipy\pandas-joris\pandas\tools\util.pyc in to_numeric(arg, er
rors, compact_ints, use_unsigned)
    135             values = lib.downcast_int64(values, {},
    136                                         use_unsigned=use_unsigned,
--> 137                                         use_sentinels=0)
    138     except Exception:
    139         if errors == 'raise':

inference.pyx in pandas.lib.downcast_int64 (pandas\lib.c:58443)()

ValueError: Buffer dtype mismatch, expected 'int64_t' but got 'long'
```

(on Windows you already get it by just passing `np.array([1,2,3])` as this gives int32)

I am not sure how difficult it would be to support other integer types, but otherwise we should probably check for this and provide a better error message.

@jorisvandenbossche : not super cryptic in fact - that's coming from the Cython (it's a type error).  However, you are right that we should allow for different `int` dtypes, so I just created a `fused_type` and threw literally all of the integer types I could think of under it.  Now your example should not raise.

Yeah, I know, not cryptic to me, but for users I meant :-)

Should we rename the function to `downcast_int` now? (or would this be used by others) 

Earlier in the thread there were some questions about the keyword argument names. Maybe we could consider `downcast` instead of `compact_ints`? 

@jorisvandenbossche : either keyword name would be fine in this case, so don't have a strong preference for one or the other, @jreback ?

@jorisvandenbossche , @jreback : tests and benchmarks have been added, and Travis is still passing

yes we use `downcast` a fair amount in the codebase as a kw arg. I think `downcast='int'` could easily subsitute here (FYI I wouldn't bother trying to downcast floats from float64->float64), BUT, you _can_ try to downcast floats if `downcast='int'` is passed (note that this is pretty much NOT done in the code base because its not cheap, you have to look for all not-null, all of this logic is already done in `common._possibly_downcast_to_dtype` btw).

 Yes this is pushing more behavior here, but this is a pretty generic function.

@jreback : Oh!  That's not how I interpreted @jorisvandenbossche question (it seemed like he was whether we could replace e.g. `compact_ints=True` with `downcast=True`).

Your interpretation is interesting though.  I think the important detail for `to_numeric` is that you can't "downcast" to any `dtype` as you can with `_possibly_downcast_to_dtype` (e.g. you wouldn't cast to `datetime` in this function).  Therefore, we would have to do validation to ensure that we were only getting numeric dtypes.

So something along the lines of:

``` python
... # validation to ensure that np.dtype(downcast) is a numeric dtype
if downcast is not None:  # maybe check to make sure that 
                                        # values.dtype != np.dtype(downcast)?
   values = common._possibly_downcast_to_dtype(values, downcast)
```

@gfyoung of course. This would be a 'public' way of doing downcasting. What I mean is the impl is easy. Its possible lots of cases are not tested as current the downcaster is all internal and called from appropriate blocks (so its implicity safe). Further since this is a `to_numeric` method I would only allow int/float for this arg (or convertible) (and now could simply have 1 arg), IOW, `downcast='int', downcast='unsigned'` for example.

As I said this is really a fancy / public way of doing astype.

Some really strange caching issues on Travis I think - there are no longer any changes to `downcast_int64`, and the attribute is definitely attached to `pandas.lib` (I've run the tests on my machine several times with no such failures).

cc @nparley 

Travis config was just changed 

Ah, okay.  Then that needs to be investigated, as I cannot reproduce those attribute errors I'm seeing on Travis.  The only explanation is that it somehow is keeping the old compiled `lib.c` files.