#5877 enhancement closed fixed (fixed)
Get twisted.python.compat to run (and import!) under Python 3.3
Reported by: | Itamar Turner-Trauring | Owned by: | Itamar Turner-Trauring |
---|---|---|---|
Priority: | normal | Milestone: | Python 3.3 Minimal |
Component: | core | Keywords: | |
Cc: | Thijs Triemstra | Branch: |
branches/compat-python3-5877
branch-diff, diff-cov, branch-cov, buildbot |
Author: | itamar |
Description
twisted.python.compat
should run, and have its tests pass, under Python 3.3
Change History (9)
comment:1 Changed 7 years ago by
Author: | → itamarst |
---|---|
Branch: | → branches/compat-python3-5877 |
comment:2 Changed 7 years ago by
Author: | itamarst → itamar |
---|---|
Keywords: | review added |
Owner: | changed from Itamar Turner-Trauring to Jean-Paul Calderone |
Ready for review (buildbot is running: http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/compat-python3-5877).
- Added PY2 and PY3 constants.
- Disabled some functionality for Python 3, temporarily, and reimplemented a tiny bit of testing code until
FilePath
andSynchronousTestCase
are available under Python 3. - Deleted tests that test code no longer present in compat, i.e. they're testing Python built-in functionality.
- Deleted some code branches that were never reached in compat.
- Marked appropriate modules and test modules as having been ported to Python 3.
Note that you should run Python 3 tests using admin/run-python3-tests
in addition to running trial.
comment:3 Changed 6 years ago by
Keywords: | review removed |
---|---|
Owner: | changed from Jean-Paul Calderone to Itamar Turner-Trauring |
Thanks! Here's a grab bag:
- There's a bunch of commented out code that I guess you meant to delete.
- The FilePath hack in
test_compat.py
should have a ticket reference (and the ticket should probably mention the presence of that hack too). In general, I think we should annotate temporary fixes like this. - Same for the mktemp hack
- And for the use of stdlib unittest
test_python3
's docstring is incorrect- The native strings all need to be changed to explicitly b"" or u"" I guess.
- I guess I sort of see the appeal of the
PY2
/PY3
(mostly in the symmetry)... but wouldn't one or the other be sufficient? Regardless, do we want these to be public? - Part of the porting strategy is not changing APIs right? Deleting inet_pton et al seems like an API change. Are we deciding that we can drop APIs outside of the normal process as part of the port? That seems like a bad idea for the same reason dropping APIs outside of the normal process the rest of the time is (ie, it makes it harder for application developers).
- And same comment about a ticket reference in
__init__.py
for the definition of the version.
comment:5 Changed 6 years ago by
Keywords: | review added |
---|---|
Owner: | changed from Itamar Turner-Trauring to Jean-Paul Calderone |
I've addressed 1-5 and 7 and 9.
Re point 8: I thought we'd discussed only partially porting certain modules, to save time. In this particular case, app developers were never even intended to use these APIs (albeit they were public); they were there for monkeypatching the socket module. So in general and in particular, dropping them as a way of speeding the port seems reasonable to me.
In other cases, we'd probably want a ticket saying "come back later and finish porting remaining parts of module XXX to Python 3". In this case, if we are going to file a ticket, it should be "Deprecate inet_pton etc. on Python 2."
Re point 6: I did a few, but AFAICT all the remaining strings are in one of three categories:
- Need to be in native format, so bytes on Python 2 and unicode on Python 3. So I can't choose a particular one. That leaves using a wrapper (
nativeString()
), which doesn't seem worth it for these, since there's no issue of potentially bad inputs as with things that end up in e.g.getattr()
from a protocol's data. - Are used in tests simply with no real semantic difference between bytes and unicode, e.g.
test_set
; I could mark them as one or the other, but it's extra work and no real benefit. - Are in code where we don't care if runs on Python 3 (
inet_pton
and related code), just that imports.
comment:6 Changed 6 years ago by
Keywords: | review removed |
---|---|
Owner: | changed from Jean-Paul Calderone to Itamar Turner-Trauring |
Re point 8: I thought we'd discussed only partially porting certain modules, to save time. In this particular case, app developers were never even intended to use these APIs (albeit they were public); they were there for monkeypatching the socket module. So in general and in particular, dropping them as a way of speeding the port seems reasonable to me.
Okay, then there should be a ticket for that deprecation, and a comment in the code linking to it. I want to avoid "temporary" Python 3 hacks that are not tracked by any tickets.
Also needs a (misc) news fragment.
Otherwise looks okay, I think. I forced a new build, to be safe. If that passes, and with the above mentioned changes, go ahead and merge.
comment:7 Changed 6 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:8 Changed 6 years ago by
Cc: | Thijs Triemstra added |
---|
Please ignore if this is Python 3.2 specific, and works on 3.3 (or isn't supposed to work at all yet) but when trying to run-python3-tests
there's this traceback now:
$ ./admin/run-python3-tests Traceback (most recent call last): File "./admin/run-python3-tests", line 17, in <module> unittest.main(module=None, argv=["run-python3-tests", "-v"] + testModules) File "/usr/lib/python3.2/unittest/main.py", line 123, in __init__ self.parseArgs(argv) File "/usr/lib/python3.2/unittest/main.py", line 191, in parseArgs self.createTests() File "/usr/lib/python3.2/unittest/main.py", line 198, in createTests self.module) File "/usr/lib/python3.2/unittest/loader.py", line 132, in loadTestsFromNames suites = [self.loadTestsFromName(name, module) for name in names] File "/usr/lib/python3.2/unittest/loader.py", line 132, in <listcomp> suites = [self.loadTestsFromName(name, module) for name in names] File "/usr/lib/python3.2/unittest/loader.py", line 91, in loadTestsFromName module = __import__('.'.join(parts_copy)) File "/home/thijs/workspaces/opensource/Twisted/trunk/twisted/test/test_compat.py", line 172 script = self.writeScript(u"foo += 1\n") ^ SyntaxError: invalid syntax
(In [35270]) Branching to 'compat-python3-5877'