ENH:Add EA types to read CSV
State: closed

Closes GH23228

- [x ] closes #23228
- [ x] tests added / passed
- [x ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x ] whatsnew - May need some  guidance on this part.


Labels:
name: IO CSV
description: read_csv, to_csv
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

Comments:
Hello @kprestel! Thanks for updating the PR.

























Cheers ! There are no PEP8 issues in this Pull Request. :beers: 

##### Comment last updated on January 02, 2019 at 02:01 Hours UTC
Where exactly in the chain of pandas' normal parse, infer, astype, chain does this fall? Is it at the very end?

This seems fragile... Consider a person writing a `DecimalArray` extension type. Would the `DecimalDtype._from_sequence` be given a list of strings? Would pandas have already converted the numeric-like values to floats, thus losing precision? We'll need to better document exactly when this is called.
If we want to support this in general for EAs, I think we need to add a new method to the interface to parse from strings. `_from_sequence` should not be expected to be able to do that. 
(and if the EA does not implement it, we can raise in `read_csv` saying this dtype does not support it)
see #23595 maybe ``_from_sequence_of_strings`` ? or a more general function
@TomAugspurger I'm nowhere near an expert in pandas but from what I can tell the casting happens at the very end. The infer/astype chain seems to start in `parsers.pyx` `self._convert_tokens` where `self == TextReader`. From just reading the code I don't think there is a risk of losing precision as you stated, although it should be tested. 

@jorisvandenbossche Are you suggesting that we add something like `_from_sequence_of_strings` to the EA interface as @jreback suggested?  I can try this and then we can see what it would look like. 

@jreback Can you explain a little bit more about how to create a new test module? I'm not exactly sure what pattern to follow. I'm guessing I need to create a test class and inherit from some base test class but I can't figure out which. Just putting my test in the new `io.py` as you suggested isn't enough because `pytest` doesn't pick it up. Sorry if this is a dumb question, I just haven't worked with a test suite like this before. 

Thanks everyone for their feedback!
> From just reading the code I don't think there is a risk of losing precision as you stated, although it should be tested.

As a quick bit of debugging, you could drop a `print(type(scalars[0]))` in `DecimalArray._from_sequence` when you refactor to use that. Though I think @jorisvandenbossche's suggestion of adding a dedicated method to API should be followed.

We should also think about this in the context of https://github.com/pandas-dev/pandas/issues/20612. There will be common patterns for every type of IO we want to add support for (no concrete suggestions right now though).
I think whatever this change ends up being it will close #20612. In the original issue (#23228), it is noted that `read_csv` is broken because it will return `object` as the `dtype` if the specified `dtype` is an EA. I believe this is due to the `_get_dtype_type` function in `dtypes/common.py`. 

Basically when you pass a EA to the `dtype` dict in `read_csv` it gets treated as a `type`. Which means `_get_dtype_type` turns it into a `np.object`. 

It seems like that needs to be fixed before anything.

@kprestel pls merge master.
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/23255?src=pr&el=h1) Report
> Merging [#23255](https://codecov.io/gh/pandas-dev/pandas/pull/23255?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/5b0610b875476a6f3727d7e9bedb90d370c669b5?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `50%`.

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

```diff
@@            Coverage Diff             @@
##           master   #23255      +/-   ##
==========================================
+ Coverage   42.46%   42.46%   +<.01%     
==========================================
  Files         161      161              
  Lines       51557    51592      +35     
==========================================
+ Hits        21892    21908      +16     
- Misses      29665    29684      +19
```

| Flag | Coverage Δ | |
|---|---|---|
| #single | `42.46% <50%> (ø)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/23255?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/dtypes/cast.py](https://codecov.io/gh/pandas-dev/pandas/pull/23255/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL2Nhc3QucHk=) | `47.63% <0%> (-0.79%)` | :arrow_down: |
| [pandas/io/parsers.py](https://codecov.io/gh/pandas-dev/pandas/pull/23255/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL3BhcnNlcnMucHk=) | `48.54% <100%> (ø)` | :arrow_up: |
| [pandas/core/series.py](https://codecov.io/gh/pandas-dev/pandas/pull/23255/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc2VyaWVzLnB5) | `50.6% <100%> (-0.15%)` | :arrow_down: |
| [pandas/core/arrays/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/23255/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2Jhc2UucHk=) | `50.98% <100%> (+0.64%)` | :arrow_up: |
| [pandas/core/arrays/integer.py](https://codecov.io/gh/pandas-dev/pandas/pull/23255/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2ludGVnZXIucHk=) | `38.27% <50%> (+0.29%)` | :arrow_up: |
| [pandas/core/dtypes/common.py](https://codecov.io/gh/pandas-dev/pandas/pull/23255/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL2NvbW1vbi5weQ==) | `69.21% <60%> (-0.69%)` | :arrow_down: |
| [pandas/core/arrays/categorical.py](https://codecov.io/gh/pandas-dev/pandas/pull/23255/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2NhdGVnb3JpY2FsLnB5) | `42.01% <0%> (+0.12%)` | :arrow_up: |
| [pandas/core/dtypes/dtypes.py](https://codecov.io/gh/pandas-dev/pandas/pull/23255/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL2R0eXBlcy5weQ==) | `76.68% <0%> (+0.51%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/23255?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/23255?src=pr&el=footer). Last update [5b0610b...3e5ec56](https://codecov.io/gh/pandas-dev/pandas/pull/23255?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/23255?src=pr&el=h1) Report
> Merging [#23255](https://codecov.io/gh/pandas-dev/pandas/pull/23255?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/681e75cfe09251be7fa01b5660a8a4281cdc5050?src=pr&el=desc) will **increase** coverage by `0.01%`.
> The diff coverage is `100%`.

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

```diff
@@            Coverage Diff             @@
##           master   #23255      +/-   ##
==========================================
+ Coverage    92.3%   92.32%   +0.01%     
==========================================
  Files         166      166              
  Lines       52412    52454      +42     
==========================================
+ Hits        48381    48430      +49     
+ Misses       4031     4024       -7
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.75% <100%> (+0.02%)` | :arrow_up: |
| #single | `43.01% <52.94%> (-0.05%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/23255?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/arrays/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/23255/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2Jhc2UucHk=) | `98.25% <100%> (+0.02%)` | :arrow_up: |
| [pandas/core/arrays/integer.py](https://codecov.io/gh/pandas-dev/pandas/pull/23255/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2ludGVnZXIucHk=) | `96.29% <100%> (+0.06%)` | :arrow_up: |
| [pandas/io/parsers.py](https://codecov.io/gh/pandas-dev/pandas/pull/23255/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL3BhcnNlcnMucHk=) | `95.4% <100%> (+0.02%)` | :arrow_up: |
| [pandas/core/arrays/timedeltas.py](https://codecov.io/gh/pandas-dev/pandas/pull/23255/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL3RpbWVkZWx0YXMucHk=) | `87.42% <0%> (-0.26%)` | :arrow_down: |
| [pandas/core/reshape/reshape.py](https://codecov.io/gh/pandas-dev/pandas/pull/23255/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcmVzaGFwZS9yZXNoYXBlLnB5) | `99.35% <0%> (-0.22%)` | :arrow_down: |
| [pandas/core/indexes/timedeltas.py](https://codecov.io/gh/pandas-dev/pandas/pull/23255/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy90aW1lZGVsdGFzLnB5) | `90.47% <0%> (-0.19%)` | :arrow_down: |
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/23255/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `87.59% <0%> (-0.17%)` | :arrow_down: |
| [pandas/core/series.py](https://codecov.io/gh/pandas-dev/pandas/pull/23255/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc2VyaWVzLnB5) | `93.64% <0%> (-0.1%)` | :arrow_down: |
| [pandas/core/indexes/period.py](https://codecov.io/gh/pandas-dev/pandas/pull/23255/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9wZXJpb2QucHk=) | `92.37% <0%> (-0.09%)` | :arrow_down: |
| [pandas/core/indexes/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/23255/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZXMucHk=) | `96.3% <0%> (-0.03%)` | :arrow_down: |
| ... and [22 more](https://codecov.io/gh/pandas-dev/pandas/pull/23255/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/23255?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/23255?src=pr&el=footer). Last update [681e75c...f42235a](https://codecov.io/gh/pandas-dev/pandas/pull/23255?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

can you merge master
@jreback Done. Sorry about the delay.
I think the implementation works and is ready for a full review. I may need a bit of guidance on the testing part though. I've tested it locally and it works but I'm not sure how to test it "more generally" following existing patterns. Any guidance on that aspect would be greatly appreciated. 

Thanks for all the help/time given to me so far!
@jreback You're right and not 20 minutes after I pushed it did I think about that... I was too focused on the use case you described in the original issue, however if I just rip out everything that isn't in `parsers.pyx` I don't think it would work at all. If it did it wouldn't work the python parser for sure. 

I will reply to your comments and answer your questions.

Thanks for the review. I really appreciate it. 


@kprestel for > https://github.com/pandas-dev/pandas/pull/23255#issuecomment-445563694

right, the python parser will need some small fixes as well. but let's start minimally.
@jreback You do need the changes in `pandas/core/dtypes/cast.py`. 

Here is an example:

```python
import pandas as pd
from pandas.compat import StringIO
from pandas.core.arrays.integer import Int64Dtype

df = pd.DataFrame({'Int': pd.Series([1, 2, 3], dtype='Int64'),
                   'A': [1, 2, 1]})
data = df.to_csv(index=False)
# this will raise a TypeError: object cannot be converted to IntegerDtype
result = pd.read_csv(StringIO(data), dtype={'Int': Int64Dtype},
                     engine='python')
# this will raise a AttributeError: 'str' object has no attribute 'construct_array_type'
result = pd.read_csv(StringIO(data), dtype={'Int': 'Int64'},
                     engine='python')
```

This example assumes you are on my branch and have reverted my changes to `pandas/core/dtypes/cast.py`


Using this same example code, if you were to revert the changes on line 158 in `pandas/core/arrays/integer.py` the first example using `Int64Dtype` will fail with this exception: 
`ValueError: invalid dtype specified <class 'pandas.core.arrays.integer.Int64Dtype'>`

This is the example from the original issue. I replied to that comment with a more detailed explanation as to why I think it should stay there, if you still think I should pull it out I will and update the necessary test cases. 




@kprestel not what I mean. you certainly need a change in ``parsers.py`` to handle dtype inference for EA for the python parser. But I don't think you need *any* changes to the actual dtype routines themselves. I would need an example that is on master.
@jreback ah, ok. I see what you mean. Sorry about the misunderstanding. But just to make sure I'm clear, you're saying what I'm doing (inferring the dtype for EAs) is correct, I'm just not doing it in the correct place? It should be done in `parsers .py`.
> @jreback ah, ok. I see what you mean. Sorry about the misunderstanding. But just to make sure I'm clear, you're saying what I'm doing (inferring the dtype for EAs) is correct, I'm just not doing it in the correct place? It should be done in parsers .py.

for the engine='c' case its fine, you just need a path for teh python parser
Ok, I made the python parser work more like the `C` parser. Should have thought to do that to begin with... Sorry about that.
@jreback I updated the tests based on what you said, let me know if it is still wrong. 

I also resolved your comments, most of them had to do with handling uninstantiated EAs. My change to `pandas_dtypes` resolved all of this, so I pulled all of the extra checks out. 

Thanks again for the feedback, I really appreciate it.   
The CI build should be fixed now. You're right changing `pandas_dtype` was a bad idea... 
lgtm. @TomAugspurger @gfyoung any comments.
@jreback Thanks again for all the help along the way! I really appreciated it.
@kprestel no, thank you! this is a really useful feature!

actually now that I think about it. Can you add an example in io.rst (and maybe in the read_csv doc-string) showing say reading in as Int64 (I think we already have an example, so you can just add another column that is Int64), actually maybe add reading as decimal would be cool too.
@jreback Done. Let me know if that isn't what you were looking for.
perfect
@kprestel some lint issues:https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=6210
ping on green.
Seems like this needs to get updated or removed? This works now I think...

2018-12-30T20:24:59.3212208Z Check for invalid EA testing
2018-12-30T20:24:59.3294654Z ##[error]pandas/tests/extension/base/io.py(24,): error :  Found unwanted pattern:         tm.assert_frame_equal(result, expected)

Otherwise it should be fine now.