#5980 enhancement closed fixed (fixed)

Port twisted/internet/abstract.py to Python 3

Reported by: exarkun Owned by: exarkun
Priority: normal Milestone: Python 3.3 Minimal
Component: core Keywords:
Cc: Branch: branches/abstract-python3-5980-3
(diff, github, buildbot, log)
Author: exarkun Launchpad Bug:

Description

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

Change History (7)

comment:1 Changed 23 months ago by exarkun

  • Author set to exarkun
  • Branch set to branches/abstract-python3-5980

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

comment:2 Changed 23 months ago by exarkun

  • Branch changed from branches/abstract-python3-5980 to branches/abstract-python3-5980-2

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

comment:3 Changed 23 months ago by exarkun

  • Keywords review added
  • Owner changed from exarkun to itamar

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 23 months ago by itamar

  • Keywords review removed
  • Owner changed from itamar to exarkun
  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 23 months ago by exarkun

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 23 months ago by exarkun

  • Branch changed from branches/abstract-python3-5980-2 to branches/abstract-python3-5980-3

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

comment:7 Changed 23 months ago by exarkun

  • Resolution set to fixed
  • Status changed from new to closed

(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.