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

Ensure Python3 before reading README with encoding arg #472

Merged
merged 2 commits into from
Nov 16, 2018

Conversation

nokome
Copy link
Contributor

@nokome nokome commented Nov 15, 2018

If you accidentally try to install using Python 2 you get this error:

    Complete output from command python setup.py egg_info:
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/home/nokome/nokome/repo2docker/setup.py", line 5, in <module>
        with open('README.md', encoding="utf8") as f:
    TypeError: 'encoding' is an invalid keyword argument for this function

Ideally, the user would be asked to use Python 3. If you remove the encoding argument, and try again then pip gives us this, more informative message:

$ python -m pip install -e .
Obtaining file:///home/nokome/nokome/repo2docker
jupyter-repo2docker requires Python '>=3.4' but the running Python is 2.7.12

This little PR implements a similar behaviour, whilst also reading the README using the correct encoding.

If you accidently try to install using Python 2 you get the error

    Complete output from command python setup.py egg_info:
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/home/nokome/nokome/repo2docker/setup.py", line 5, in <module>
        with open('README.md', encoding="utf8") as f:
    TypeError: 'encoding' is an invalid keyword argument for this function

Ideally the user would be aked to use Python 3 as `python_requires` tells us,
but which isn't reached yet.
This implements that behaviour.
@betatim
Copy link
Member

betatim commented Nov 15, 2018

Seems like a good thing to do.

One pondering: should we make the reading of the README conditional on the Python version? For the case were we are on Python 2 we can set it to an empty string and then let pip generate the error message. The advantage would be that it shows the actual minimum required (currently 3.4 but maybe in the future we start requiring something newer and then "Python 3" isn't good enough anymore).

@nokome
Copy link
Contributor Author

nokome commented Nov 15, 2018

One pondering: should we make the reading of the README conditional on the Python version?

Yes, that's a better approach! I'll modify.

@nokome
Copy link
Contributor Author

nokome commented Nov 15, 2018

Now, when trying to install with Python 2 gives the error:

> python -m pip install -e .
Obtaining file:///home/nokome/nokome/repo2docker
jupyter-repo2docker requires Python '>=3.4' but the running Python is 2.7.12

Copy link
Member

@betatim betatim left a comment

Choose a reason for hiding this comment

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

Merge away once the robots are happy.

@betatim betatim merged commit 1cef296 into jupyterhub:master Nov 16, 2018
markmo pushed a commit to markmo/repo2docker that referenced this pull request Jan 22, 2021
markmo pushed a commit to markmo/repo2docker that referenced this pull request Jan 22, 2021
Ensure Python3 before reading README with encoding arg
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.

None yet

2 participants