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

switch from open to io.open for text file read/write #3769

Merged

Conversation

indirectlylit
Copy link
Contributor

@indirectlylit indirectlylit commented Jun 7, 2018

Summary

In learningequality/kolibri-installer-windows#72 we noticed that characters aren't being rendered properly in Windows using Python 3

This PR attempts to find all instances of open that were used on text files, and replace them with io.open, using an explicit encoding.

Reviewer guidance

Please check that this all makes sense

References


Contributor Checklist

  • Contributor has fully tested the PR manually
  • PR has the correct target branch and milestone
  • PR has 'needs review' or 'work-in-progress' label
  • If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • If there are any front-end changes, before/after screenshots are included
  • If this is an important user-facing change, PR or related issue has a 'changelog' label

Reviewer Checklist

  • Automated test coverage is satisfactory
  • Reviewer has fully tested the PR manually
  • PR has been tested for accessibility regressions
  • External dependencies files were updated (yarn and pip)
  • Documentation is updated
  • Link to diff of internal dependency change is included
  • CHANGELOG.rst is updated for high-level changes
  • Contributor is in AUTHORS.rst

@indirectlylit indirectlylit added the TODO: needs review Waiting for review label Jun 7, 2018
@indirectlylit indirectlylit added this to the 0.10.0 milestone Jun 7, 2018
@codecov
Copy link

codecov bot commented Jun 8, 2018

Codecov Report

Merging #3769 into release-v0.10.x will decrease coverage by <.01%.
The diff coverage is 70.58%.

Impacted file tree graph

@@                 Coverage Diff                 @@
##           release-v0.10.x    #3769      +/-   ##
===================================================
- Coverage            76.74%   76.74%   -0.01%     
===================================================
  Files                  215      215              
  Lines                 8991     8999       +8     
  Branches              1110     1110              
===================================================
+ Hits                  6900     6906       +6     
- Misses                1856     1858       +2     
  Partials               235      235
Impacted Files Coverage Δ
kolibri/utils/conf.py 84.21% <ø> (ø) ⬆️
kolibri/auth/management/commands/importusers.py 97.75% <ø> (ø) ⬆️
...bri/content/management/commands/generate_schema.py 0% <0%> (ø) ⬆️
...bri/logger/management/commands/generateuserdata.py 85.71% <100%> (+0.52%) ⬆️
kolibri/auth/constants/facility_presets.py 90% <100%> (+2.5%) ⬆️
kolibri/content/utils/stopwords.py 100% <100%> (ø) ⬆️
kolibri/content/hooks.py 77.14% <100%> (+0.67%) ⬆️
kolibri/core/webpack/hooks.py 66.48% <50%> (+0.17%) ⬆️

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 9d143db...d91a9af. Read the comment docs.

@indirectlylit
Copy link
Contributor Author

hmm looks like it's not building... checking on it

@mrpau-richard
Copy link
Contributor

mrpau-richard commented Jun 8, 2018

@indirectlylit the fix confirmed at the Windows 7. Non-english characters are now displaying properly.
screen shot 2018-06-08 at 11 21 54 am

@indirectlylit
Copy link
Contributor Author

great!

might also help with #2541

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TODO: needs review Waiting for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants