BUG: Fix encoding for Stata format 118 files

[bashtage]

Ensure that Stata 118 files always use utf-8 encoding

• closes BUG: read_stata always uses 'utf8' #21244
• tests added / passed
• passes git diff upstream/master -u -- "*.py" | flake8 --diff
• whatsnew entry

[codecov]

Codecov Report

Merging #21279 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #21279   +/-   ##
=======================================
  Coverage   91.85%   91.85%           
=======================================
  Files         153      153           
  Lines       49549    49549           
=======================================
  Hits        45512    45512           
  Misses       4037     4037

Flag	Coverage Δ
#multiple	90.25% <ø> (ø)	⬆️
#single	41.87% <ø> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b32fdc4...ac8c2c2. Read the comment docs.

[pep8speaks]

Hello @bashtage! Thanks for updating the PR.

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

Comment last updated on June 05, 2018 at 07:20 Hours UTC

[jorisvandenbossche]

I know you say it didn't have any effect before, but still, we should not introduce deprecation warnings in a bug fix release. So would either move this to 0.24.0 or split the PR

[bashtage]

I removed the deprecation.

[jreback]

@bashtage so the fix looks good. can your restore the encoding paramater then can merge into 0.23.1 (and then come back with another PR to deprecate it)? thxs

[bashtage]

I removed the deprecation warning but encoding is now a no-op, which is the correct behavior.

I'd rather not restore the (potentially) broken behavior where users are allowed to set an invalid encoding (e.g. latin-1 for more recent file formats), and this can wait to 24 if you think this is too much change for a bug fix release.

[jorisvandenbossche]

I'd rather not restore the (potentially) broken behavior where users are allowed to set an invalid encoding (e.g. latin-1 for more recent file formats), and this can wait to 24 if you think this is too much change for a bug fix release.

Yes, we should not remove the keyword for 0.23.1, even when it was a no-op, because that breaks people code who inadvertently used it. So I would add it back here, but then a next PR to deprecate it if you want.

[bashtage]

The keyword is still available in read_Stata and Stata Reader and so code won't break.

…

On Wed, Jun 6, 2018, 13:54 Joris Van den Bossche ***@***.***> wrote: I'd rather not restore the (potentially) broken behavior where users are allowed to set an invalid encoding (e.g. latin-1 for more recent file formats), and this can wait to 24 if you think this is too much change for a bug fix release. Yes, we should not remove the keyword for 0.23.1, even when it was a no-op, because that breaks people code who inadvertently used it. So I would add it back here, but then a next PR to deprecate it if you want. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#21279 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFU5Rahuwy53qij-xodhPX9G9cdjgDq3ks5t59DugaJpZM4UV4zG> .

[jorisvandenbossche]

Ah, sorry, missed that you only removed the internal passing through of the keyword. That's fine then!

[jorisvandenbossche]

@bashtage Thanks!