Skip to content

Open README.rst with utf-8 encoding#651

Merged
pganssle merged 3 commits into
dateutil:masterfrom
gergondet:fix_non_utf8_setup
Mar 26, 2018
Merged

Open README.rst with utf-8 encoding#651
pganssle merged 3 commits into
dateutil:masterfrom
gergondet:fix_non_utf8_setup

Conversation

@gergondet

Copy link
Copy Markdown
Contributor

With the current setup script, the build fails (I only tested with Python 3.4) in a non-UTF-8 environment

e.g. LC_ALL=C pip3 install . fails prior to this patch

This fails because open uses the system encoding by default and the README.rst file contains non-ascii characters.

This PR enforces the opening of README.rst with the UTF-8 encoding. This fixes the build when running the setup script on non-UTF-8 setups.

Comment thread setup.py Outdated

def README():
with open('README.rst') as f:
with open('README.rst', encoding='utf-8') as f:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think open does not take this keyword in Python 2.7.

io.open is the same as Python 3's open in both 2.7 and 3. That will fix this.

@pganssle

pganssle commented Mar 26, 2018

Copy link
Copy Markdown
Member

@gergondet Thanks for this PR. Good catch!

If you are not already in the Authors file, can you add yourself there as part of the PR?

@gergondet

gergondet commented Mar 26, 2018

Copy link
Copy Markdown
Contributor Author

@pganssle I've changed open to io.open and added my self to the authors file, thanks for being so reactive 🍰

@pganssle
pganssle force-pushed the fix_non_utf8_setup branch from f5acf8c to 1c6952c Compare March 26, 2018 13:17
@pganssle

Copy link
Copy Markdown
Member

Awesome, thanks @gergondet. I squashed the fixup commit into the original README commit just for a cleaner history, but once CI passes I'll merge.

Thanks for this PR. I should cut a new release with it relatively soon so that people installing from source with non-UTF-8 environments don't end up broken.

@pganssle
pganssle merged commit dc809ea into dateutil:master Mar 26, 2018
@pganssle pganssle mentioned this pull request Mar 26, 2018
@pganssle pganssle added this to the 2.7.2 milestone Mar 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants