Skip to content

Conversation

@gergondet
Copy link
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.

setup.py Outdated

def README():
with open('README.rst') as f:
with open('README.rst', encoding='utf-8') as f:
Copy link
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
Copy link
Member

pganssle commented Mar 26, 2018

@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
Copy link
Contributor Author

gergondet commented Mar 26, 2018

@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
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