Added explicit encoding to all open statements.
#407
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added `encoding="utf-8"` to all calls to `open` that open a text-based file or stream, where encoding was not already specified. This is intended to deal with inconsistencies that arise from not speciyfing or inconsistently specifying encoding on platforms where utf-8 is not necessarily the default text file encoding.
phackstock
reviewed
Oct 4, 2024
There was a problem hiding this comment.
Thanks a lot @korsbakken for catching and fixing this oversight.
With regards to Excel, we should be fine since the information about the encoding used should be part of the file itself.
Since we run all of our tests in GitHub on Windows, MacOS and Ubuntu you can just add a test for reading a character that would require utf-8. This way we can verify that it at least works on the GitHub actions version of Windows. It might also be interesting to see if we can reproduce the error in a GH action.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added
encoding="utf-8"to all calls toopenthat open a yaml file or stream, where encoding was not already specified. This is intended to deal with inconsistencies that arise from not specifying or inconsistently specifying encoding on platforms where utf-8 is not necessarily the default text file encoding.I have only checked for calls to
open. There might be other types of calls that could be affected by platform-dependent encoding defaults and that don't specify encoding, such as calls throughpyamorpandasmethods, code that deals with Excel files. But especially in the latter case, it's maybe less clear that we should enforce UTF-8.This PR addresses #406 and potentially resolves it. Unfortunately I don't have a suitable Windows system to test on.