Opened 4 years ago

Closed 4 years ago

#5980 enhancement closed fixed (fixed)

Port twisted/internet/abstract.py to Python 3

Reported by: Jean-Paul Calderone Owned by: Jean-Paul Calderone
Priority: normal Milestone: Python 3.3 Minimal
Component: core Keywords:
Cc: Branch: branches/abstract-python3-5980-3
branch-diff, diff-cov, branch-cov, buildbot
Author: exarkun

Description

Basis of all reactor implementations. Needed to get very far on those.

Change History (7)

comment:1 Changed 4 years ago by Jean-Paul Calderone

Author: exarkun
Branch: branches/abstract-python3-5980

(In [35649]) Branching to 'abstract-python3-5980'

comment:2 Changed 4 years ago by Jean-Paul Calderone

Branch: branches/abstract-python3-5980branches/abstract-python3-5980-2

(In [35669]) Branching to 'abstract-python3-5980-2'

comment:3 Changed 4 years ago by Jean-Paul Calderone

Keywords: review added
Owner: changed from Jean-Paul Calderone to Itamar Turner-Trauring

Maybe not 100% working yet, but better covered now, the marked tests do pass, and at least the thorny issue of buffer is kinda addressed.

Build results

comment:4 Changed 4 years ago by Itamar Turner-Trauring

Keywords: review removed
Owner: changed from Itamar Turner-Trauring to Jean-Paul Calderone
  1. I assume you removed the doWrite return value of 0 on purpose? That does mean the docstring is no longer accurate; it should be updated.
  2. Did you make sure no reactor relies on that return value? (I hope not...)
  3. memoryview (also available in 2.7), seems like a suitable replacement for buffer. If it is, file a ticket to switch to using that when available, and then we don't need to have Python 3-specific code. Or just do it here, whichever is easier. Possibly separate ticket is better, since we'd want to check if it's significantly slower on 2.7.
  4. Minor merge conflict; "solvable" by point below, maybe.
  5. I feel that we shouldn't be adding twisted.internet.abstract to ported list until we've actually ported a few more test modules and have a bit more assurance it works. Only really important if other people end up working on this, but seems like good idea to be clear about what is _really_ ported.

(As a side note, we should start exploring using bytearrays now that we've upgraded minimal requirement to 2.6.)

Please address the above, then merge.

comment:5 Changed 4 years ago by Jean-Paul Calderone

I assume you removed the doWrite return value of 0 on purpose? That does mean the docstring is no longer accurate; it should be updated.

Yes. Okay. r35726.

Did you make sure no reactor relies on that return value? (I hope not...)

I can't check all reactors, of course. None of our reactors depend on the difference between 0 and None though, and the interface specifies None be returned.

memoryview (also available in 2.7), seems like a suitable replacement for buffer. If it is, file a ticket to switch to using that when available, and then we don't need to have Python 3-specific code. Or just do it here, whichever is easier. Possibly separate ticket is better, since we'd want to check if it's significantly slower on 2.7.

memoryview is apparently useless for this case - http://bugs.python.org/issue15945

I feel that we shouldn't be adding twisted.internet.abstract to ported list until we've actually ported a few more test modules and have a bit more assurance it works. Only really important if other people end up working on this, but seems like good idea to be clear about what is _really_ ported.

Sure. r35727.

comment:6 Changed 4 years ago by Jean-Paul Calderone

Branch: branches/abstract-python3-5980-2branches/abstract-python3-5980-3

(In [35729]) Branching to 'abstract-python3-5980-3'

comment:7 Changed 4 years ago by Jean-Paul Calderone

Resolution: fixed
Status: newclosed

(In [35733]) Merge abstract-python3-5980-3

Author: exarkun Reviewer: itamar Fixes: #5980

Some basic steps towards having twisted.internet.abstract work on Python 3. Syntax changes and missing test coverage is the focus here. Some simplification of the doWrite implementation, too.

Note: See TracTickets for help on using tickets.