Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#14075 closed enhancement (implemented)

Unified codebase for Python 2&3

Reported by: Foxboron Owned by: atagar
Priority: Medium Milestone:
Component: Core Tor/Stem Version:
Severity: Keywords: python, 3, 2, unified, codebase
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

https://github.com/Foxboron/stem/commit/7899e3b23d1a44af56bc0fdf05a2fb20f3ec884c

This basically unifies the codebase and removes the need for a tool like 2to3.
The most instrusive change is the need for having a unified understanding of long/int/str/bytes/unicode across version.

Python 2.7 understands bytes to be str, so this will be consistent, but unicode is undefined on Python3.

So _compat.py solves this by defining long to be int, and unicode to be str on python3. This so far solves the problem in a nice way. But needs to be kept tidy when writing new code that relies on string types. Currently you can refer to bytes to be bytes, but have to import from _compat unicode if you want to refer to unicode, as references to "str" is unique to the python version.
This can be made easier to refer to a unified variable, but that could be further discussed.

This will most likely break 2.6 support, but i can work on adding that if thats actually needed.

Child Tickets

Change History (9)

comment:1 Changed 3 years ago by Foxboron

Keywords: python 3 2 unified codebase added

comment:2 Changed 3 years ago by atagar

Hi Foxboron, this is great! I started poking around it and first thoughts are...

  1. All of the dict's keys(), values(), and items() calls are now wrapped by list(). I know 2to3 does this, and I suspect it's maybe to have a defensive copy. But most of our calls to this are in for loops where it's not necessary.
  1. I see the unit tests work, but the integ tests don't. It's most due to unicode strings with 'u'. I'm guessing that you didn't try the integ tests?

comment:3 Changed 3 years ago by Foxboron

Helluw!

  1. Actually, we do need this. Stem does pass around a lot of dicts being changes in threads. If we remove the forced evaluation we wind up with RuntimeError: dictionary changed size during iteration, i suspect this is because of possibly "bad" code in other places, but i wanted this change to be as non-intrusive as possible on the old code. Nailing these kind of bugs can be a possible PR later. My opinion on the matter though.
  1. Right, only tested with python 3.4, i can solve this and maintain compatibility with 2.6, and 3.x as well. Me being short sighted.

comment:4 Changed 3 years ago by Foxboron

Work work!

2.6 support:
https://github.com/Foxboron/stem/commit/95c2187563cb194afa4aa4ec2d479614da91e291
Did some error handling for the mock library, also replaced the OrderedDict we added
in an earlier commit as it's not really needed with the upcomming changes to the tests.

3.x support:
https://github.com/Foxboron/stem/commit/e1047c8173f08504a90779f518919e387e29fb9e

Please note that i was unable to compile 3.1, so i haven't actually tested it, tho
i'm very sure it should work without any major problems.

comment:5 Changed 3 years ago by atagar

Fantastic! All the targets with both python2 and python3 passed for me so I'll look this over tomorrow. Nice work!

comment:6 Changed 3 years ago by atagar

Resolution: implemented
Status: newclosed

comment:7 Changed 3 years ago by atagar

Oh oops, forgot to ask - are you ok with your patches being under the public domain? It's been a while since I've received a patch that was big enough to need to ask, but this certainly fits the bill. ;)

https://stem.torproject.org/faq.html#what-is-the-copyright-for-patches

comment:8 Changed 3 years ago by Foxboron

All good with me!

comment:9 Changed 3 years ago by atagar

Great! Again, thanks for the patch. :)

Note: See TracTickets for help on using tickets.