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

Remove f-strings #520

Merged
merged 3 commits into from Dec 18, 2018

Conversation

@jrbourbeau
Copy link
Contributor

jrbourbeau commented Dec 18, 2018

This PR removes f-strings from repo2docker

There are a few f-strings in the codebase that are causing the RTD build to fail because the docs are currently built with Python 3.5, while f-strings weren't added until 3.6 (ref https://readthedocs.org/projects/repo2docker/builds/8291787/). I was able to reproduce the f-string related SyntaxError when building the docs locally on master in a Python 3.5 environment. With the changes in this PR, the docs were successfully built locally with Python 3.5.

jrbourbeau added 3 commits Dec 18, 2018
@betatim

This comment has been minimized.

Copy link
Member

betatim commented Dec 18, 2018

In the README.md we write that repo2docker supports Python 3.4+. We should update that to reflect what we actually support and then add a test to check we can run on that version.

JupyterHub supports 3.5+, in the absence of strong opinions should we go with that?

(I don't remember if we have discussed this before or not. If yes please link us to that so we don't have to rediscuss this.)

@minrk

This comment has been minimized.

Copy link
Member

minrk commented Dec 18, 2018

Our travis.yml looks like it's trying to add a test entry for Python 3.4. We should check why that's not getting included.

@yuvipanda yuvipanda merged commit bdd32b2 into jupyter:master Dec 18, 2018
4 checks passed
4 checks passed
ci/circleci: build_docs Your tests passed on CircleCI!
Details
codecov/patch 66.66% of diff hit (target 20%)
Details
codecov/project 84.97% remains the same compared to 36641f3
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@yuvipanda

This comment has been minimized.

Copy link
Collaborator

yuvipanda commented Dec 18, 2018

Thanks for the PR, @jrbourbeau! I opened #521 to talk about supported python versions.

@jrbourbeau jrbourbeau deleted the jrbourbeau:remove-f-strings branch Dec 18, 2018
@jrbourbeau

This comment has been minimized.

Copy link
Contributor Author

jrbourbeau commented Dec 18, 2018

FYI, the RTD build is passing now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.