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

Dropped support for Python 2 #1954

Merged
merged 11 commits into from May 15, 2020
Merged

Dropped support for Python 2 #1954

merged 11 commits into from May 15, 2020

Conversation

tgaddair
Copy link
Collaborator

Fixes #1929.

  • Removed Python 2 tests from Buildkite.
  • Removed references to Python 2 in Docker images.
  • Removed system version checks that perform different logic for Python 2 vs Python 3.
  • Removed dependency on six and replaced all usages with native Python 3 equivalents.
  • Removed doc references to Python 2 and added Python 3 to install requirements.
  • Removed unit test exclusions for Python 2.
  • Removed Python 2 from supported versions in setup.

@EnricoMi
Copy link
Collaborator

Do we need to mention Python3 in docs somewhere?

@tgaddair
Copy link
Collaborator Author

Do we need to mention Python3 in docs somewhere?

I added a mention in the Install Guide under "requirements". Might be worth it to put it somewhere on the main README, but couldn't think of a good place.

Signed-off-by: Travis Addair <taddair@uber.com>
Signed-off-by: Travis Addair <taddair@uber.com>
Signed-off-by: Travis Addair <taddair@uber.com>
Signed-off-by: Travis Addair <taddair@uber.com>
Signed-off-by: Travis Addair <taddair@uber.com>
Signed-off-by: Travis Addair <taddair@uber.com>
Signed-off-by: Travis Addair <taddair@uber.com>
Signed-off-by: Travis Addair <taddair@uber.com>
Signed-off-by: Travis Addair <taddair@uber.com>
.buildkite/gen-pipeline.sh Outdated Show resolved Hide resolved
.buildkite/gen-pipeline.sh Outdated Show resolved Hide resolved
@EnricoMi
Copy link
Collaborator

Do we need to mention Python3 in docs somewhere?

I added a mention in the Install Guide under "requirements". Might be worth it to put it somewhere on the main README, but couldn't think of a good place.

I think install.rst is good enough, haven't seen that before I send that comment.

@EnricoMi
Copy link
Collaborator

Are these imports due to Python 2 compatibility?

try:
    from shlex import quote
except ImportError:
    from pipes import quote
try:
    from collections.abc import Iterable
except ImportError:
    from collections import Iterable

@tgaddair
Copy link
Collaborator Author

Are these imports due to Python 2 compatibility?

try:
    from shlex import quote
except ImportError:
    from pipes import quote
try:
    from collections.abc import Iterable
except ImportError:
    from collections import Iterable

Good catch, looks like these imports both work in Python 3.6 and above, so I'll update.

Signed-off-by: Travis Addair <taddair@uber.com>
Signed-off-by: Travis Addair <taddair@uber.com>
@tgaddair tgaddair merged commit 842d107 into master May 15, 2020
@tgaddair tgaddair deleted the no-py2 branch May 15, 2020 23:06
@EnricoMi
Copy link
Collaborator

EnricoMi commented May 16, 2020

There is one more Python2 condition in .buildkite/gen-pipeline.sh: https://github.com/horovod/horovod/blob/master/.buildkite/gen-pipeline.sh#L261. Created #1958 to fix that and some lost integration tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Proposal: Drop Python 2 support in v0.20.0
3 participants