API/BUG: Handling Dtype Coercions in Series/Index (GH 15832)
State: closed

 - [x] closes #15832
 - [x] tests added / passed
 - [x] passes ``git diff upstream/master --name-only -- '*.py' | flake8 --diff``
 - [x] whatsnew entry

Hey @jreback , need a quick help here. I was able to raise an overflow exception (case II), but I wasn't able to raise it on case III. Can you help?  Thanks

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

Comments:
> I was able to raise an overflow exception (case II), but I wasn't able to raise it on case III.

Seems like you were able to successfully do that.

Also, don't forget to make similar patches to `Index`!  And perhaps squash your commits when you have time so that the contributions and changes are a lot cleaner.
> And perhaps squash your commits when you have time so that the contributions and changes are a lot cleaner.

This is not necessary. We will squash when merging. If it makes it cleaner for you, then sure,

@ucals did you push your changes?
Ops, forgot to push.... Pushing it right away, 1 sec
This should actually work. We allow casting to a compat dtype when there is no loss of precision.
so if you have a passed int/unsigned int with a float, then you cast to int, if it works you allow it to proceed. We already do this for indexes: https://github.com/pandas-dev/pandas/blob/master/pandas/indexes/numeric.py#L146. 

```
In [1]: Series([1., 2., 3.], dtype='int')
Out[1]: 
0    1
1    2
2    3
dtype: int64

```

The counter example of course are:


This should raises (as this loses precision).
```
In [5]: Series([1, 1.5],dtype='int')
Out[5]: 
0    1
1    1
dtype: int64
```

This is good. (pls just confirm that our error messages are the same for Index/Series).
```
In [6]: Series([1, np.nan],dtype='int')
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-6-b52843b42d56> in <module>()
----> 1 Series([1, np.nan],dtype='int')

/Users/jreback/pandas/pandas/core/series.py in __init__(self, data, index, dtype, name, copy, fastpath)
    242             else:
    243                 data = _sanitize_array(data, index, dtype, copy,
--> 244                                        raise_cast_failure=True)
    245 
    246                 data = SingleBlockManager(data, index, fastpath=True)

/Users/jreback/pandas/pandas/core/series.py in _sanitize_array(data, index, dtype, copy, raise_cast_failure)
   2856         if dtype is not None:
   2857             try:
-> 2858                 subarr = _try_cast(data, False)
   2859             except Exception:
   2860                 if raise_cast_failure:  # pragma: no cover

/Users/jreback/pandas/pandas/core/series.py in _try_cast(arr, take_fast_path)
   2810             subarr = maybe_cast_to_datetime(arr, dtype)
   2811             if not is_extension_type(subarr):
-> 2812                 subarr = np.array(subarr, dtype=dtype, copy=copy)
   2813         except (ValueError, TypeError):
   2814             if is_categorical_dtype(dtype):

ValueError: cannot convert float NaN to integer
```
Hey @jreback , sorry for the past days, got overloaded in my work. But now I can resume the work on this PR. I'm having some difficulties with the code, can you help me? Specifically:
1) the `(d2<0).any()` makes a test break
2) if I don't use `any` in `is_float_dtype(type([1, 2, 3.5]))`, I can't make it work
3) I tried to implement the `_assert_safe_casting` for series as you pointed out we already do for indexes, but couldn't manage to make it work.

Can you help me? Pls let me know (I can submit the code as is now, all tests are ok and your other comments are ok as well)
@ucals make sure everything is pushed up (looks like you updated comments, but code is older I think). I will take a look tomorrow.
Hey @jreback , after trying and changing a lot over the past days, I'm starting over, fresh from master... will do a push today
can you rebase / update
Sure, @jreback , I will push current status tomorrow
Hey @jreback , just pushed my code, but I still need some help. First, after a lot of tentatives, I rolled back and started again, fresh clean. Then, I added the failing tests to tackle the issue (GH 15832) and wrote the code to pass them. So, far, so good... Then, the code broke 2 other tests, and I'm not being able to successfully make the right adjustments to the code to make them pass.
Here's the output of the 2 failing tests: [output.txt](https://github.com/pandas-dev/pandas/files/1004762/output.txt)

Can you help? Thanks!

@ucals what you are changing is quite tricky. looks like there are some valid cases which you are now raising on. you need to differentiate between those cases.
@jreback good news: I managed to fix the first issue. Now it only raises if it try to coerce if there are negative integers and dtype is unsigned integer. But still need help to fix the 2nd issue. How to check if coercion loses precision? The coercion of [1 2 3.5] is raising (and that's right), but the coercion of [1. 1. 8.] is also raising, but it should not - as there's no loss of precision. Any ideas?
@jreback another good news: I manage to fix the 2nd issue! I figured out that how to check if coercion from float to int loses precision: just compare the original array with another one that we convert to int and then to float :)
Now all tests are passing! 
Actually, not passing.. Now it's breaking in 9 different places, let me check..
I think I figured it out... finally :)
Hey @jreback , did all your comments.. New tests added to this issue are passing. But other tests from other parts of the code are breaking. Can you help?
Thanks!
u need the debug
maybe cases r missed or not handled certain cases

we have lots of tests that r effectively integration tests so they exercise cases that may not be hit by the unit tests 
Hey @jreback , I managed to make all tests pass in my machine. It passes in Circle CI, in AppVeyor, and in 6 out of 9 of Travis CI. However, it fails in 3 Travis CI, but I can't reproduce them in my machine. Take a look below:

![image](https://cloud.githubusercontent.com/assets/19690196/26275395/cde34972-3d35-11e7-94b5-9d6e29425d60.png)

https://travis-ci.org/ucals/pandas/builds/234242837?utm_source=email&utm_medium=notification

How can I fix it? Can you help?

Thanks!

[This](https://travis-ci.org/pandas-dev/pandas/jobs/234242857#L1376) has some linting failures in

```
- pandas/core/series.py:17:1: F401 'pandas.core.dtypes.common.is_unsigned_integer_dtype' imported but unused
- pandas/tests/indexing/test_coercion.py:290:80: E501 line too long (82 > 79 characters)
- pandas/tests/indexing/test_coercion.py:597:9: E265 block comment should start with '# '
```

Don't worry about [this](https://travis-ci.org/pandas-dev/pandas/jobs/234242861) failure. We have a flaky test on that CI run, and I haven't been able to debug it yet

For the third one, the doc build stalled. I've seen that once before, not sure what's up. I restarted it.
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/15859?src=pr&el=h1) Report
> Merging [#15859](https://codecov.io/gh/pandas-dev/pandas/pull/15859?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/42e2a87f2a8848795238de1259a3daa5612e393d?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/15859/graphs/tree.svg?token=eZ4WkYLtcO&src=pr&width=650&height=150)](https://codecov.io/gh/pandas-dev/pandas/pull/15859?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #15859      +/-   ##
==========================================
+ Coverage    90.4%   90.41%   +<.01%     
==========================================
  Files         161      161              
  Lines       50933    51010      +77     
==========================================
+ Hits        46048    46120      +72     
- Misses       4885     4890       +5
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `88.25% <100%> (+0.08%)` | :arrow_up: |
| #single | `40.16% <69.23%> (-0.16%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/15859?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/indexes/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/15859?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9iYXNlLnB5) | `95.76% <100%> (+0.01%)` | :arrow_up: |
| [pandas/core/dtypes/cast.py](https://codecov.io/gh/pandas-dev/pandas/pull/15859?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL2Nhc3QucHk=) | `87.06% <100%> (+0.16%)` | :arrow_up: |
| [pandas/core/series.py](https://codecov.io/gh/pandas-dev/pandas/pull/15859?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc2VyaWVzLnB5) | `94.72% <100%> (ø)` | :arrow_up: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/15859?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `25% <0%> (-58.34%)` | :arrow_down: |
| [pandas/io/common.py](https://codecov.io/gh/pandas-dev/pandas/pull/15859?src=pr&el=tree#diff-cGFuZGFzL2lvL2NvbW1vbi5weQ==) | `69.91% <0%> (-0.3%)` | :arrow_down: |
| [pandas/io/packers.py](https://codecov.io/gh/pandas-dev/pandas/pull/15859?src=pr&el=tree#diff-cGFuZGFzL2lvL3BhY2tlcnMucHk=) | `88.23% <0%> (-0.28%)` | :arrow_down: |
| [pandas/io/pytables.py](https://codecov.io/gh/pandas-dev/pandas/pull/15859?src=pr&el=tree#diff-cGFuZGFzL2lvL3B5dGFibGVzLnB5) | `93.06% <0%> (-0.17%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/15859?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.69% <0%> (-0.1%)` | :arrow_down: |
| [pandas/io/stata.py](https://codecov.io/gh/pandas-dev/pandas/pull/15859?src=pr&el=tree#diff-cGFuZGFzL2lvL3N0YXRhLnB5) | `93.47% <0%> (-0.09%)` | :arrow_down: |
| [pandas/io/formats/format.py](https://codecov.io/gh/pandas-dev/pandas/pull/15859?src=pr&el=tree#diff-cGFuZGFzL2lvL2Zvcm1hdHMvZm9ybWF0LnB5) | `95.66% <0%> (ø)` | :arrow_up: |
| ... and [12 more](https://codecov.io/gh/pandas-dev/pandas/pull/15859?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/15859?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/15859?src=pr&el=footer). Last update [42e2a87...86e9d5e](https://codecov.io/gh/pandas-dev/pandas/pull/15859?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/15859?src=pr&el=h1) Report
> Merging [#15859](https://codecov.io/gh/pandas-dev/pandas/pull/15859?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/ef487d9e474e8052c0f7c6260de5802a950defad?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/15859/graphs/tree.svg?src=pr&width=650&token=eZ4WkYLtcO&height=150)](https://codecov.io/gh/pandas-dev/pandas/pull/15859?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #15859      +/-   ##
==========================================
+ Coverage   90.79%    90.8%   +<.01%     
==========================================
  Files         161      161              
  Lines       51063    51076      +13     
==========================================
+ Hits        46365    46378      +13     
  Misses       4698     4698
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `88.64% <100%> (ø)` | :arrow_up: |
| #single | `40.16% <64.28%> (ø)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/15859?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/indexes/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/15859?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9iYXNlLnB5) | `95.75% <100%> (ø)` | :arrow_up: |
| [pandas/core/series.py](https://codecov.io/gh/pandas-dev/pandas/pull/15859?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc2VyaWVzLnB5) | `94.91% <100%> (ø)` | :arrow_up: |
| [pandas/core/dtypes/cast.py](https://codecov.io/gh/pandas-dev/pandas/pull/15859?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL2Nhc3QucHk=) | `87.06% <100%> (+0.16%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/15859?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/15859?src=pr&el=footer). Last update [ef487d9...b78f4cc](https://codecov.io/gh/pandas-dev/pandas/pull/15859?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Hey @TomAugspurger , thanks for the support! Just pushed the code fixing the linting problems. :)
Cheers!
@jreback , @TomAugspurger , all checks have passed!!
Hey @jreback , thanks for the comments. I just implemented all your comments, with exception of "assert that this works with float32/float64". Can you pls show me how?
Also, the code is breaking 1 test in `test_pytables.py`, and I don't know how to fix yet.
> Hey @jreback , thanks for the comments. I just implemented all your comments, with exception of "assert that this works with float32/float64". Can you pls show me how?
Also, the code is breaking 1 test in test_pytables.py, and I don't know how to fix yet.

If we are going to raise on [3], then assert that [2] does not.
```
In [2]: s = Series([1, 2.5, 3], dtype='float64')

In [3]: s
Out[3]: 
0    1.0
1    2.5
2    3.0
dtype: float64

In [5]: s = Series([1, 2.5, 3], dtype='int64')

In [6]: s
Out[6]: 
0    1
1    2
2    3
dtype: int64
```
Just double-checking: looking in the issue, I understood that `pd.Index([1, 2, 3.5], dtype=int)` should raise. What about `pd.Series([1, 2, 3.5], dtype=int)`? It shouldn't, right?
> Just double-checking: looking in the issue, I understood that pd.Index([1, 2, 3.5], dtype=int) should raise. What about pd.Series([1, 2, 3.5], dtype=int)? It shouldn't, right?


would be weird if these didn't act the same
Yes, that's what I thought. That's why we wrote in `pandas/tests/series/test_constructors.py`:
```python
# GH 15832
for t in ['uint8', 'uint16', 'uint32', 'uint64']:
    with pytest.raises(ValueError):
        Series([1, 2, 3.5], dtype=t)
```
This test passes. However, it was written in `pandas/tests/io/test_pytables.py`:
```python
# check with mixed dtypes
df1 = DataFrame(dict([(c, Series(np.random.randn(5), dtype=c))
    for c in ['float32', 'float64', 'int32', 'int64', 'int16', 'int8']]))
```
If I don't change this test to expect a ValueError for the 4 int types, it will never pass, because it's the same case. These 2 cases are the same. How to differentiate those?

Thanks
Carlos