ENH: 'encoding_errors' argument for read_csv/json
State: closed

- [x] closes #39450 
- [x] closes #39017
- [x] tests added / passed
- [x] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [x] whatsnew entry

Should `encoding_errors` be added to `to_csv` and should `errors` in `to_csv` show a `DeprecationWarning` (for consistent naming)?

Labels:
name: Unicode
description: Unicode strings
name: IO CSV
description: read_csv, to_csv
name: IO JSON
description: read_json, to_json, json_normalize

Comments:
I think `errors` is fine since it aligns with the stdlib. I do remember a similar conversation where I asked for something besides `errors` as a parameter, but I think that was scoped to very rare cases with old `.xls` file types which was a little different
> I think `errors` is fine since it aligns with the stdlib. I do remember a similar conversation where I asked for something besides `errors` as a parameter, but I think that was scoped to very rare cases with old `.xls` file types which was a little different

hmm my concern here is `errors` could mean errros in the lines themselves

though we have 
```
   error_bad_lines=True,
    warn_bad_lines=True,
```
for these.
Ah that’s a valid point. In that case I _do_ think something like `encoding_errors` is explicit, so maybe the best option


I don't understand why the C-engine doesn't work. I think it either a) gets the original byte sequence and cannot convert it to UTF-8 or b) it gets the corrected string ("corrected" = "surrogatepass") but it doesn't like that?

edit: probably the second case as `"replace"` seems to work.
Is the error happening in the tokenizer perhaps? You could try to enable more verbose debugging of that if you uncomment this line and rebuild the extensions:

https://github.com/pandas-dev/pandas/blob/316f5acce16cc4407c54c5c939222e544a231f65/pandas/_libs/src/parser/tokenizer.h#L45

It will give a lot of details so will want to have a minimal example
It doesn't print much for `pytest -s -k test_encoding_surrogatepass[c_high] pandas/tests/io/parser/common/test_common_basic.py`:

```
_tokenize_helper: Asked to tokenize 2 rows, datapos=0, datalen=0
parser_buffer_bytes self->cb_io: nbytes=262144, datalen: 13083248, status=0
free_if_not_null 0x20bef00
free_if_not_null (nil)
free_if_not_null 0x2476220
free_if_not_null 0x254dce0
free_if_not_null 0x24c3b10
free_if_not_null 0x255bd30
free_if_not_null 0x24bccc0
UnicodeEncodeError: 'utf-8' codec can't encode character '\udf7f' in position 0: surrogates not allowed
```

`\udf7f` is the string after `surrogatepass`. Maybe my test just uses a weird example? Based on the error it seems that the c-engine tries to encode `\udf7f` (convert the string back to bytes) and fails during that - but it shouldn't need to do that (at least the python engine doesn't do that).
Debugged a little bit and noticed that the C extensions fails very early on before tokenization even happens. This branch gets hit when just trying to interact with the file handle:

https://github.com/pandas-dev/pandas/blob/10034a48f383574d78524b48eff9ae78b80ae358/pandas/_libs/src/parser/io.c#L188

Not sure why the `read()` call is failing a few lines up but providing in case helpful
thank you for looking into it! I don't "feel at home" debugging the c extensions ;) but I will try to stare a bit more at it.

I assume
```
args = Py_BuildValue("(i)", nbytes);
func = PyObject_GetAttrString(src->obj, "read");
result = PyObject_CallObject(func, args);
```
should correspond to
```
obj = open(...)  # done in python
obj.read(nbytes)
```
I'm surprised it tries to read a specific amount of bytes (maybe the variable name is just misleading), the C-engine is always given a string buffer.
The file should already be opened at this point - this part of the C code is really just calling read on the handle. I *think* the issue is before the extension even gets called

Sent from my iPhone

> On Feb 25, 2021, at 4:56 PM, Torsten Wörtwein <notifications@github.com> wrote:
> 
> ﻿
> thank you for looking into it! I don't "feel at home" debugging the c extensions ;) but I will try to stare a bit more at it.
> 
> I assume
> 
> args = Py_BuildValue("(i)", nbytes);
> func = PyObject_GetAttrString(src->obj, "read");
> result = PyObject_CallObject(func, args);
> should correspond to
> 
> obj = open(...)  # done in python
> obj.read(nbytes)
> I'm surprised it tries to read a specific amount of bytes (maybe the variable name is just misleading), the C-engine is always given a string buffer.
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or unsubscribe.

Ah sorry I was wrong before. I think the issue actually stems from this line in the extension

https://github.com/pandas-dev/pandas/blob/859a78729c3b670136093b749912c2d752664203/pandas/_libs/src/parser/io.c#L194

According to the Python docs the error handling for that function is always "strict"

https://docs.python.org/3/c-api/unicode.html?highlight=pyunicode_asutf8string#c.PyUnicode_AsUTF8String

Changing that line to `tmp = PyUnicode_AsEncodedString(result, "utf-8", "surrogatepass");` at least moves the failure elsewhere (obviously wouldn't want to hard code the errors argument, but providing now as demo)
OK this diff should get your test to pass:

```sh
diff --git a/pandas/_libs/parsers.pyx b/pandas/_libs/parsers.pyx
index c4d98ccb8..c403eaf18 100644
--- a/pandas/_libs/parsers.pyx
+++ b/pandas/_libs/parsers.pyx
@@ -632,7 +632,7 @@ cdef class TextReader:
             char *word
             object name, old_name
             uint64_t hr, data_line = 0
-            char *errors = "strict"
+            char *errors = "surrogatepass"
             StringPath path = _string_path(self.c_encoding)
             list header = []
             set unnamed_cols = set()
@@ -673,11 +673,8 @@ cdef class TextReader:
                 for i in range(field_count):
                     word = self.parser.words[start + i]
 
-                    if path == UTF8:
-                        name = PyUnicode_FromString(word)
-                    elif path == ENCODED:
-                        name = PyUnicode_Decode(word, strlen(word),
-                                                self.c_encoding, errors)
+                    name = PyUnicode_Decode(word, strlen(word),
+                                            self.c_encoding, errors)
 
                     # We use this later when collecting placeholder names.
                     old_name = name
diff --git a/pandas/_libs/src/parser/io.c b/pandas/_libs/src/parser/io.c
index 51504527d..6abce4ce5 100644
--- a/pandas/_libs/src/parser/io.c
+++ b/pandas/_libs/src/parser/io.c
@@ -191,7 +191,7 @@ void *buffer_rd_bytes(void *source, size_t nbytes, size_t *bytes_read,
         *status = CALLING_READ_FAILED;
         return NULL;
     } else if (!PyBytes_Check(result)) {
-        tmp = PyUnicode_AsUTF8String(result);
+      tmp = PyUnicode_AsEncodedString(result, "utf-8", "surrogatepass");
         Py_DECREF(result);
         if (tmp == NULL) {
             PyGILState_Release(state);
```

These are just hard coded arguments so will need to figure out a way to pass them along through the parser, but should hopefully give you the gist of what we need to do.

Separately I think we might want to audit our uses of `PyUnicode_FromString` and replace with `PyUnicode_Decode` if we wanted to have generic support for the errors argument. A lot of the factorization in the parser module seem to be using the former which I think just defaults to strict error handling
thank you @WillAyd very very much! That is super helpful!

I will look into how I can best incorporate that :)
Now it should forward `encding_errors` :) I replaced only enough `PyUnicode_FromString` with `PyUnicode_Decode` to get (most) tests working.

When running `pytest -k test_encoding_surrogatepass[c_high] pandas/tests/io/parser/common/test_common_basic.py`
`self.encoding_errors` becomes b'' at some time after the `TextReader` is initialized.
I think the C / Cython edits are reasonable. It looks like the test is failing due to passing keyword arguments from Python down to Cython. I think this is because encoding_errors isn't specified as an option in pandas/io/parsers/readers.py
`encoding_errors` is parsed as part of the kwargs. Adding print() into the cython code shows that it is passed to the ctython code. I think there might be something wrong with either setting the `encoding_errors` attribute (it has the intended value while being in the constructor but afterwards it it empty/pointing to a random array) or some other code resets it after the constructor.
could it be that the attribute is garbage collected after the constructor because no pure python code references it?
Actually looking at this again I think you need to update the declarations in tokenizer.h that are being imported into parsers.pyx . There might be some mangling of the signature since the header files don’t match what is actually being referenced
This makes it work (at least locally for me):
https://github.com/pandas-dev/pandas/pull/39777/commits/e19a0c69e653b595ffd94cbc89f892b88a7b3790#diff-09751231cb113bbb35c3bf29ffe2302664d76cd6667aa3edf44f3d886f083bacR382-R383
but I assume this is not an elegant solution.  If I had to guess why it work is that python now knows that there is still a reference to the `encoding_errors` object.
I think I should call `Py_DECREF` but calling `Py_DECREF(PyBytes_FromString(self.encoding_errors))` in `close()` causes a segfault when calling `Py_DECREF` the first time. But printing `self.encoding_errors` before `Py_DECREF` prints the expected content.
I don’t think you need to reference count methods that are cimported from Cython. Will try to look later today or tomorrow, but this is really close

Sent from my iPhone

> On Mar 2, 2021, at 1:37 PM, Torsten Wörtwein <notifications@github.com> wrote:
> 
> ﻿
> I think I should call Py_DECREF but calling Py_DECREF(PyBytes_FromString(self.encoding_errors)) in close() causes a segfault when calling Py_DECREF the first time. But printing self.encoding_errors before Py_DECREF prints the expected content.
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or unsubscribe.

lgtm. do need to update anything in : https://pandas.pydata.org/pandas-docs/dev/user_guide/io.html#io-read-csv-table ?

cc @WillAyd if any more comments.
thanks @twoertwein very nice!
> not an easy change so kudos for seeing this through

@WillAyd Thank you very much for your help! I would have been too frustrated without your help :)