Repository: pandas-dev/pandas
Pull request: BUG: Help python csv engine read binary buffers
The file buffer given to read_csv could have been opened in
binary mode, but the python csv reader errors on binary buffers.

- [x] closes #23779
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Problem statement:
read_csv c engine accepts binary mode data and python engine rejects it
#### Code Sample

```python
import pandas as pd

if __name__ == "__main__":
    with open('test.csv', 'w') as f:
        f.write('1,2,3\n4,5,6')
    with open('test.csv', 'rt') as f:
        pd.read_csv(f, header=None)
    with open('test.csv', 'rb') as f:
        pd.read_csv(f, header=None)
    with open('test.csv', 'rt') as f:
        pd.read_csv(f, header=None, engine='python')
    with open('test.csv', 'rb') as f:
        pd.read_csv(f, header=None, engine='python')
```
#### Problem description

The second read_csv call (using the C engine and a file opened in binary mode) will correctly read the csv. The fourth read_csv call (using the Python engine and a file opened in binary mode) will throw an exception stating it needs to be in text mode:

```
pandas.errors.ParserError: iterator should return strings, not bytes (did you open the file in text mode?)
```

Perhaps this is intended behavior, but I found this difference in behavior between the engines surprising, as well as that binary mode was accepted at all.

#### Expected Output

Either the C engine rejecting binary mode files or the Python engine accepting them.

#### Output of ``pd.show_versions()``

<details>
INSTALLED VERSIONS
------------------
commit: None
python: 3.6.6.final.0
python-bits: 64
OS: Linux
OS-release: 4.15.0-39-generic
machine: x86_64
processor: x86_64
byteorder: little
LC_ALL: None
LANG: en_US.UTF-8
LOCALE: en_US.UTF-8

pandas: 0.23.4
pytest: None
pip: 10.0.1
setuptools: 39.1.0
Cython: None
numpy: 1.15.4
scipy: None
pyarrow: None
xarray: None
IPython: None
sphinx: None
patsy: None
dateutil: 2.7.5
pytz: 2018.7
blosc: None
bottleneck: None
tables: None
numexpr: None
feather: None
matplotlib: None
openpyxl: None
xlrd: None
xlwt: None
xlsxwriter: None
lxml: None
bs4: None
html5lib: None
sqlalchemy: None
pymysql: None
psycopg2: None
jinja2: None
s3fs: None
fastparquet: None
pandas_gbq: None
pandas_datareader: None
</details>

Hints from issue discussion:
cc @gfyoung 
@rgeens : Thanks for opening this!  Sorry that this got lost in the pile of issues that we have 😞 

I strongly believe that this discrepancy is symptomatic of limitations in Python's native `csv` library.  The Python engine is largely a wrapper around it, and the error message you're showing comes from the `csv` module in fact.

To illustrate my point:

~~~python
import csv

with open('test.csv', 'w') as f:
    f.write('1,2,3\n4,5,6')

r = csv.reader(open('test.csv', 'rb'))
print(next(r))
~~~

This outputs:

~~~
...
_csv.Error: iterator should return strings, not bytes (did you open the file in text mode?)
~~~

This is beyond our control, so the best we can do is to test the behavior for the C engine (if it doesn't exist already).  You're more than welcome to do that!
I would like to have this reopened, because it's not actually beyond pandas control to fix this cleanly and because right now the read_csv function doc specifies "file-like object" not "file-like object opened in ascii mode".

Just conditionally wrap the object in a io.TextIOWrapper, since you _know_ that csv is by definition an ascii format:
```Python
import csv
import io

def __ascii_wrap(potentially_binary_buffer):
    try:
        return io.TextIOWrapper(potentially_binary_buffer)
    except Exception:
        return potentially_binary_buffer


with open('test.csv', 'w') as f:
    f.write('1,2,3\n4,5,6')

r = csv.reader(__ascii_wrap(open('test.csv', 'rb')))
print(next(r))
```
@fiendish : That's a good point.  You're more than welcome to open a PR to implement this.

Commit message: black formatting

Patch to review:
diff --git a/doc/source/whatsnew/v1.0.0.rst b/doc/source/whatsnew/v1.0.0.rst
index aeed3668fe774..39e640dcc4935 100644
--- a/doc/source/whatsnew/v1.0.0.rst
+++ b/doc/source/whatsnew/v1.0.0.rst
@@ -157,7 +157,7 @@ MultiIndex
 I/O
 ^^^
 
--
+- :meth:`read_csv` now accepts binary mode file buffers when using the Python csv engine (:issue:`23779`)
 -
 
 Plotting
diff --git a/pandas/io/common.py b/pandas/io/common.py
index e01e473047b88..37d7cb988e46b 100644
--- a/pandas/io/common.py
+++ b/pandas/io/common.py
@@ -5,7 +5,7 @@
 import csv
 import gzip
 from http.client import HTTPException  # noqa
-from io import BytesIO
+from io import BufferedIOBase, BytesIO
 import lzma
 import mmap
 import os
@@ -342,9 +342,11 @@ def _get_handle(
     try:
         from s3fs import S3File
 
-        need_text_wrapping = (BytesIO, S3File)
+        need_text_wrapping = (BufferedIOBase, BytesIO, S3File)
     except ImportError:
-        need_text_wrapping = (BytesIO,)
+        need_text_wrapping = (BufferedIOBase, BytesIO)
+
+    no_close = BufferedIOBase
 
     handles = list()
     f = path_or_buf
@@ -420,8 +422,10 @@ def _get_handle(
     if is_text and (compression or isinstance(f, need_text_wrapping)):
         from io import TextIOWrapper
 
-        f = TextIOWrapper(f, encoding=encoding, newline="")
-        handles.append(f)
+        g = TextIOWrapper(f, encoding=encoding, newline="")
+        if not isinstance(f, no_close):
+            handles.append(g)
+        f = g
 
     if memory_map and hasattr(f, "fileno"):
         try:
diff --git a/pandas/tests/io/parser/test_common.py b/pandas/tests/io/parser/test_common.py
index b94d5cd497ccf..daae92a1e5c8d 100644
--- a/pandas/tests/io/parser/test_common.py
+++ b/pandas/tests/io/parser/test_common.py
@@ -2020,9 +2020,32 @@ def test_file_handles_with_open(all_parsers, csv1):
     # Don't close user provided file handles.
     parser = all_parsers
 
-    with open(csv1, "r") as f:
-        parser.read_csv(f)
-        assert not f.closed
+    for mode in ["r", "rb"]:
+        with open(csv1, mode) as f:
+            parser.read_csv(f)
+            assert not f.closed
+
+
+@pytest.mark.parametrize(
+    "fname,encoding",
+    [
+        ("test1.csv", "utf-8"),
+        ("unicode_series.csv", "latin-1"),
+        ("sauron.SHIFT_JIS.csv", "shiftjis"),
+    ],
+)
+def test_binary_mode_file_buffers(all_parsers, csv_dir_path, fname, encoding):
+    # gh-23779: Python csv engine shouldn't error on files opened in binary.
+    parser = all_parsers
+
+    fpath = os.path.join(csv_dir_path, fname)
+
+    with open(fpath, mode="r", encoding=encoding) as fa:
+        df_a = parser.read_csv(fa)
+    with open(fpath, mode="rb") as fb:
+        df_b = parser.read_csv(fb, encoding=encoding)
+
+    tm.assert_frame_equal(df_b, df_a)
 
 
 def test_invalid_file_buffer_class(all_parsers):


Code review comments:
diff_hunk: @@ -342,9 +342,11 @@ def _get_handle(
     try:
         from s3fs import S3File
 
-        need_text_wrapping = (BytesIO, S3File)
+        need_text_wrapping = (BufferedIOBase, BytesIO, S3File)
path: pandas/io/common.py
text: @user1:
BytesIO is a subclass of BufferedIOBytes. Can BytesIO be removed?

@author:
👍