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 #224

Closed
leandro-lucarella-sociomantic opened this issue Sep 12, 2017 · 12 comments · Fixed by #279
Closed

Port to Python3 #224

leandro-lucarella-sociomantic opened this issue Sep 12, 2017 · 12 comments · Fixed by #279

Comments

@leandro-lucarella-sociomantic
Copy link
Contributor

Python 2 will be retired in 2020. Yes, is still 2.5 years to go, but eventually this will have to happen.

As a start, we should use more and more from __future__ import * to have a gradual and painless migration.

This is a very good resource for this: http://python-future.org/

@Mic92
Copy link

Mic92 commented Dec 22, 2019

We probably going to remove it from nixpkgs at some point because of python2 dependency.

@joshtriplett
Copy link

This is now becoming an issue in Debian, as well.

@llucax
Copy link
Member

llucax commented Aug 15, 2020

Bumping priority to high. It shouldn't be hard to get this done, I'll try to allocate some time to do it.

@bokchan
Copy link
Contributor

bokchan commented Aug 18, 2020

Tried porting using 2to3, (https://docs.python.org/3.7/library/2to3.html).
Fixes the imports for python 3

Wrt adapting the Makefile, would a change as below be sufficient/sensible

export PYTHON ?= python3
...
	sed -i 's|^#!/usr/bin/env python3$$|#!/usr/bin/env $(PYTHON)|' \
			$(DESTDIR)$(prefix)/bin/git-hub

Do we wanna go full python 3 and add type annotations as well?

@llucax
Copy link
Member

llucax commented Aug 18, 2020

Tried porting using 2to3, (https://docs.python.org/3.7/library/2to3.html).
Fixes the imports for python 3

Cool, thanks!

Do we wanna go full python 3 and add type annotations as well?

In my experience type annotations in Python 3 are still a bit in the early stages and not all libraries implement them. Also, for small script like this they don't provide so much, usually larger codebases benefit more from static typing. Because of this I have almost zero personal interest in adding annotations, but I'd be happy to merge a good PR adding them iff it also adds a type checker (like http://mypy-lang.org/) to the CI, so we make sure those type annotations are actually used for something, as the Python interpreter just ignores them for now.

@bokchan
Copy link
Contributor

bokchan commented Aug 18, 2020

Comment from git-hub with unicode chars ★ ☹ ⚓

@Mic92
Copy link

Mic92 commented Aug 18, 2020

Your script

Tried porting using 2to3, (docs.python.org/3.7/library/2to3.html).
Fixes the imports for python 3

Cool, thanks!

Do we wanna go full python 3 and add type annotations as well?

In my experience type annotations in Python 3 are still a bit in the early stages and not all libraries implement them. Also, for small script like this they don't provide so much, usually larger codebases benefit more from static typing. Because of this I have almost zero personal interest in adding annotations, but I'd be happy to merge a good PR adding them iff it also adds a type checker (like http://mypy-lang.org) to the CI, so we make sure those type annotations are actually used for something, as the Python interpreter just ignores them for now.

Your script seems to only use the stdlib, which is fully supported by typeshed. Which means you can get fully typechecking coverage. But even if you are missing out on some dependencies it is still a great help when refactoring internal code in my experience.

@bokchan
Copy link
Contributor

bokchan commented Aug 19, 2020

Forgot to write a release note. But I assume this will be the main feature for v2.0.0 so might make sense to cook it up at the same time as the release.

@llucax
Copy link
Member

llucax commented Aug 20, 2020

No, I think it would be better to add it now as usual :)

@bokchan
Copy link
Contributor

bokchan commented Aug 20, 2020

No, I think it would be better to add it now as usual :)

Oki. I will create an MR for that

bokchan added a commit to bokchan/git-hub that referenced this issue Aug 23, 2020
bokchan added a commit to bokchan/git-hub that referenced this issue Aug 23, 2020
bokchan added a commit to bokchan/git-hub that referenced this issue Aug 23, 2020
bokchan added a commit to bokchan/git-hub that referenced this issue Aug 27, 2020
llucax pushed a commit that referenced this issue Aug 30, 2020
@llucax
Copy link
Member

llucax commented Oct 30, 2020

Done in #279.

@llucax llucax closed this as completed Oct 30, 2020
@llucax llucax linked a pull request Oct 30, 2020 that will close this issue
@llucax llucax modified the milestones: Future, v2.0.0 Oct 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants