Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open all text files with UTF-8 encoding #373

Merged
merged 1 commit into from
May 8, 2023
Merged

Open all text files with UTF-8 encoding #373

merged 1 commit into from
May 8, 2023

Conversation

jwodder
Copy link
Collaborator

@jwodder jwodder commented May 5, 2023

Calling open() without an explicit encoding uses the OS encoding, which is not always sensible. See PEP 597 et alii.

@jwodder jwodder added cleanup internal Changes only affect the internal API labels May 5, 2023
@codecov-commenter
Copy link

codecov-commenter commented May 5, 2023

Codecov Report

Merging #373 (18abbc2) into master (1472357) will increase coverage by 0.04%.
The diff coverage is 13.63%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master     #373      +/-   ##
==========================================
+ Coverage   33.59%   33.64%   +0.04%     
==========================================
  Files          51       51              
  Lines        7030     7030              
  Branches     1784     1784              
==========================================
+ Hits         2362     2365       +3     
+ Misses       4488     4485       -3     
  Partials      180      180              
Impacted Files Coverage Δ
src/nidm/experiment/Query.py 72.00% <0.00%> (ø)
src/nidm/experiment/Utils.py 27.29% <0.00%> (ø)
src/nidm/experiment/tools/bidsmri2nidm.py 0.00% <0.00%> (ø)
src/nidm/experiment/tools/csv2nidm.py 0.00% <ø> (ø)
src/nidm/experiment/tools/nidm2bids.py 0.00% <0.00%> (ø)
...nidm/experiment/tools/nidm_affinity_propagation.py 0.00% <0.00%> (ø)
.../experiment/tools/nidm_agglomerative_clustering.py 0.00% <0.00%> (ø)
src/nidm/experiment/tools/nidm_convert.py 0.00% <0.00%> (ø)
src/nidm/experiment/tools/nidm_gmm.py 0.00% <0.00%> (ø)
src/nidm/experiment/tools/nidm_kmeans.py 0.00% <0.00%> (ø)
... and 4 more

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@@ -639,7 +639,7 @@ def cluster_number():
# ask for help: how does one do a dendrogram, also without graphing?

if o is not None:
f = open(o, "a")
f = open(o, "a", encoding="utf-8", encoding="utf-8")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one too many ;)

Suggested change
f = open(o, "a", encoding="utf-8", encoding="utf-8")
f = open(o, "a", encoding="utf-8")

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@yarikoptic
Copy link
Member

well, some of those files should not even be utf8 but AFAIK should be ascii... but I guess it would generally be more robust etc to just generally assume UTF-8 these days.

@yarikoptic yarikoptic merged commit b0042db into master May 8, 2023
@yarikoptic yarikoptic deleted the open-utf8 branch May 8, 2023 16:39
@github-actions
Copy link

🚀 PR was released in v4.0.0 🚀

@github-actions github-actions bot added the released This issue/pull request has been released. label May 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup internal Changes only affect the internal API released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants