BUG: Fix handling of encoding for the StataReader #21244
State: closed

- [x] closes #21244
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Labels:
name: Unicode
description: Unicode strings
name: IO Stata
description: read_stata, to_stata

Comments:
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/21246?src=pr&el=h1) Report
> Merging [#21246](https://codecov.io/gh/pandas-dev/pandas/pull/21246?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/c85ab083919b59ce84c220d5baf7d34ff4a0bcf2?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.

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

```diff
@@           Coverage Diff           @@
##           master   #21246   +/-   ##
=======================================
  Coverage   91.84%   91.84%           
=======================================
  Files         153      153           
  Lines       49538    49538           
=======================================
  Hits        45499    45499           
  Misses       4039     4039
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.24% <ø> (ø)` | :arrow_up: |
| #single | `41.87% <ø> (ø)` | :arrow_up: |


------

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

@bashtage can you have a look
You need to add a small Stata produce dta file that can replicate the issue and that this or fixes. Then please add a test that reads this dta file. It needs to be produced by Stata and not pandas. 
118 does support utf8. But this needs to be tested using a real Stata file with both fixed width utf8 strings and StrL utf8 characters, and numbers. 
I see. I thought that https://github.com/pandas-dev/pandas/blob/master/pandas/tests/io/data/stata14_118.dta contains Unicode, since this test `TestStata.test_read_dta18` from https://github.com/pandas-dev/pandas/blob/master/pandas/tests/io/test_stata.py has a check for that and it passes.
Though indeed I don't know if the generated file was made by Stata or not.

Unfortunately I don't own a copy of the software and it's neither in my _field_ to work with stata files as a Scientist/Statistician. :)
My use case is for a converter for a simple view of the file.
It possible the standard strings might be latin-1 not unicode. Are you sure
that the reader doesn't already read utf8 without passing an encoding? The
dta 118 spec says all strings are unicode and so this suggests that the
encoding is always utf8 irrespective of the one passed in. If so, then the
reader shouldn't be changed and a note should be added that encoding is
ignored for 118 files.

More generally I think reader should remove encoding and always use latin-1
for dta <118 and utf8 for 118.

On Wed, May 30, 2018, 13:14 Adrian Castravete <notifications@github.com>
wrote:

> I see. I thought that
> https://github.com/pandas-dev/pandas/blob/master/pandas/tests/io/data/stata14_118.dta
> contains Unicode, since the test passes with unicode characters. And the
> TestStata.test_read_dta18 test from
> https://github.com/pandas-dev/pandas/blob/master/pandas/tests/io/test_stata.py
> has a check for that.
> Though indeed I do not know if the generated file was made by Stata or not.
>
> Unfortunately I don't own a copy of the software and it's neither in my
> *field* to work with stata files as a Scientist/Statician. :)
> My use case is for a converter for a simple view of the file.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/21246#issuecomment-393123217>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AFU5Rc-jrlC4-5xNhTVXzwoz0Zpbx14eks5t3n8KgaJpZM4URdVo>
> .
>

When I tried encoding the string with unicode and then decoding it with `latin-1` the string was different, It showed two weird characters instead of the expected `Ü`.

As for the `encoding` argument: I agree if the specs say that 118 is always Unicode then it would make sense to add the necessary ifs where applicable. I've seen parts of the code where the encoding gets set to `latin-1`. 

If the encoding is to be determined via the version number, then this argument should also be removed and everything else like _documentation_ or _tests_ should be changed to reflect this.
I took a look at the dta spec and it is stricter than pandas enforces. dta < 118 claim to use ASCII only although Stata internally displays and works with latin-1.  dta 118 is utf-8 only.
I see. I will continue with the added suggestions.
@adrian-castravete Master just got updated with a fix for this. If you have a chance could you try it out with your dta file?  It should always use the correct encoding (automatically) now. If it fails, we might need to look into your dta file.
I think this has been resolved in master.
deprecated encoding by #21400 (and bug is already fixed in master)