Skip to content
This repository has been archived by the owner on Dec 20, 2023. It is now read-only.

Python 3 support #169

Merged
merged 5 commits into from Feb 24, 2017
Merged

Python 3 support #169

merged 5 commits into from Feb 24, 2017

Conversation

renekliment
Copy link
Member

@renekliment renekliment commented Feb 6, 2017

Issue this PR fixes (if applicable): #7

Description:
This adds py3 support for the code. Not the install yet and it doesn't make it default either (you can't choose it actually). This will be available in the next version. We need to test this through first.

It doesn't work yet completely, but it it almost does.

There's also an annoying urllib3 warning, which is safe to ignore: urllib3/urllib3#800

Checklist:

  • PR Location (usually master for hotfixes, dev for general development)

@renekliment
Copy link
Member Author

renekliment commented Feb 10, 2017

Okay, this works now.

@alexa-pi/team Please have a look at this. This is the other of the two PRs that will be the last for the v1.4 release. Once this is merged, all new code will have to be python 2.7 and 3.4+ compliant.

@lechat
Copy link
Contributor

lechat commented Feb 11, 2017

There is a library, six, that handles differences between 2.7 and 3. I think that it is better to use that library instead of handling that manually

@renekliment
Copy link
Member Author

@lechat Well, we have a few ugly imports, but other than that the code is actually cleaner by this change - like not using deprecated syntax, not declaring variables textual, when they actually store objects, not using implicit relative imports, which might be confusing, ... and I don't see a need for the library then. It just seems that if we keep the code clean, it works under both python 2 & 3.

What do you think of this PR itself? Where exactly would you rather use the library?

@renekliment
Copy link
Member Author

Also, I think the code should be primarily thought as python 3 code and supporting python 2 as something extra. And using external libraries as wrappers so we can support py2 goes against it.

Even though py2 is said to be supported until 2020, py3 has been here for ages and if we should think about something, it should be dropping py2 support in order to use py3-only features. Python 3 has been in Debian and even OpenWRT/LEDE for some time so I can't think of a good reason to support py2. But since there is no special feature in py3 that we absolutely need, we should keep py2 support so people can use it if they want to for some weird reason. And if we decide to drop py2 support, we just delete those weird try import things and have clean code, instead rewriting stuff to drop the six library.

@lechat
Copy link
Contributor

lechat commented Feb 11, 2017

I disagree that 2.7 is optional and everybody shall jump to 3.x, but let's not start a flame war about it. :)

@renekliment
Copy link
Member Author

@lechat Haha, didn't mean to start one 😄 Anyways, I'm for keeping the 2.7 support as I've said. People just might have reasons now, even though I don't share them :)

Also, if you have some time, it would be awesome, if you could look if the two open PRs are okay, so we can release v1.4 asap. Thank you.

@renekliment
Copy link
Member Author

14-day PR review window rule ... merging.

@alexa-pi/team Okay, this is the last PR for v1.4! To keep the development going and align with the calendar 😄 I'm giving until the end of the month for testing and I'll release the v1.4 on the 1st of March 2017. Also, since it's just 5 days, I seriously doubt anyone will have a merge-ready PR in the meantime, so I'm not creating a branch for it. Therefore anyone who wants to test should test the dev branch.

@renekliment renekliment merged commit b424bc1 into alexa-pi:dev Feb 24, 2017
@renekliment renekliment deleted the py3 branch February 24, 2017 07:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants