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

Port to python3 #29

Closed
bhipple opened this issue Mar 22, 2020 · 0 comments · Fixed by #37
Closed

Port to python3 #29

bhipple opened this issue Mar 22, 2020 · 0 comments · Fixed by #37

Comments

@bhipple
Copy link
Contributor

bhipple commented Mar 22, 2020

NixOS/nixops has moved to python3 on the master branch, which means it no longer builds with the nixops-aws plugin (see NixOS/nixops#1258). A workaround is to checkout an older NixOps, but we should start the migration here as well so that the master branches work with each other.

This issue tracks work items/issues/blockers/progress for python3.

bhipple added a commit to bhipple/nixops-aws that referenced this issue Mar 22, 2020
Just requires fixing the print function, which works in python2 as well.
This fixes one of the stand-alone scripts for NixOS#29.
bhipple added a commit to bhipple/nixops-aws that referenced this issue Mar 22, 2020
The cmp() function was removed in python3 [1], and while there is a workaround
listed, in this particular case we only need the `max` value and that's it.
Aside from being simpler and py2/py3 compatible, also happens to be more performant.

Partial work item for NixOS#29. Note that `2to3` *cannot* fix this particular one,
hence why it's being sent in a separate PR for code review.

[1] https://docs.python.org/3.0/whatsnew/3.0.html#ordering-comparisons
bhipple added a commit to bhipple/nixops-aws that referenced this issue Mar 22, 2020
This runs two more fixes that are safe to merge in python2 for continued
progress towards NixOS#29:

1. Converting `except T, R` to `except T as R`. Both work in python2, but only
   the latter works in python3.
   https://docs.python.org/3.0/whatsnew/3.0.html#changed-syntax

2. Converting module imports to relative imports.
bhipple added a commit to bhipple/nixops-aws that referenced this issue Mar 22, 2020
In python3, `dict.items()` and `dict.values()` are iterators by defualt, so the
`iteritems` functions have been removed. In python2, these return a copy of the
keys, which has small runtime penalty but is unlikely to be material for NixOps.

Changing it to just `.items()` makes us both py2 and py3 compatible and prepares
us for the migration NixOS#29.
bhipple added a commit to bhipple/nixops-aws that referenced this issue Mar 22, 2020
In py2, filter returned a list; in py3, it returns an iterator, which can't be indexed.

Per upstream guidelines [1], while it is possible to just add a `list()` cast,
it's more idiomatic to use a list comprehension for filtering. This is more
pythonic regardless, and still py2 compatible.

Partial progress towards NixOS#29

[1] https://docs.python.org/3.0/whatsnew/3.0.html#views-and-iterators-instead-of-lists
bhipple added a commit to bhipple/nixops-aws that referenced this issue Mar 22, 2020
In py2, map returned a list; in py3, it returns an iterator, which can't be indexed.

Per upstream guidelines [1], while it is possible to just add a list() cast,
it's more idiomatic to use a list comprehension for mapping. This is more
pythonic regardless, and still py2 compatible.

Partial progress towards NixOS#29

CC @grahamc @AmineChikhaoui

[1] https://docs.python.org/3.0/whatsnew/3.0.html#views-and-iterators-instead-of-lists
bhipple added a commit to bhipple/nixops-aws that referenced this issue Mar 22, 2020
Casting to a list is a no-op in py2, but in py3 we cannot index [0] without it.

Partial progress towards NixOS#29.

This commit also removes unused imports from test case files as a general cleanup.
bhipple added a commit to bhipple/nixops-aws that referenced this issue Mar 22, 2020
This does a 1-way migration to python3, and fully resolves NixOS#29.

Aside from the `release.nix`, there are no manual changes in this PR. It was
generated with:

    2to3 -n -w nixopsaws/**/*.py -x dict -x print
    black .

We have already fixed the `print` function in a previous PR, as well as all of
the dictionary iterators. `2to3` is not particularly smart about this and will
add a `list()` cast just about everywhere otherwise.
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 a pull request may close this issue.

1 participant