Merge `twisted/internet/_utilspy3.py` into `twisted/internet/utils.py`

[Jean-Paul Calderone]

Also merge the associated test module. See #6183.

[Josh Schneier]

NativeStringIO was having a method called on it, extra periods I didn't delete it

[Josh Schneier]

i'm not sure why the tests didn't pick up the failure but patch2 is more correct. (is there a way to remove previously submitted patches?)

[Josh Schneier]

files werent removed because i did rm instead of svn rm, fixed now

[Tom Prince]

(In [37385]) Branching to internet-utils-mergs-6240.

[Tom Prince]

Thanks for your contribution.

1. You shouldn't try to port the rest of the module in this ticket, other than the minimal syntax changes to get the file to import.
   • That is, you should conventionalize the import of StringIO on _PY3, rather than changing the import.
2. For the things that aren't ported, you need to make sure that they aren't accessible. See the if _PY3: blocks at the end of twisted/python/util.py and twisted/python/test/test_util.py to see how to do this.

Please resubmit for review after addressing the above points.

(Note for future reviewers: I've done the initial dance to restore history, as in #6235 but haven't applied the above patch)

[Josh Schneier]

changes requested made

[Tom Prince]

1. reraise needs to be imported unconditionally. (Apparently there isn't test coverage for this unfortunately)
2. admin/_twistedpython3.py needs to be updated to include the merged files.
3. (minor) Please keep the import like:

   #!
   try:
       import cStringIO as StringIO
   except ImportError:
       import StringIO
   

   (as it was before merging) and don't do any import on _PY3.

I'll fix these up and submit for review. (I want somebody to review the svn merge dance as well).

[Josh Schneier]

I'm confused, can you explain why?

1. I looked through twisted.internet.utils and didn't find any hint of reraise why would I want to still import it in general?
2. I removed the _utilspy3 ones and didn't put the utils and test_iutils in. Per your last comment I shouldn't try and port the files as a whole, so I removed them from the ported list.
3. I tried to do just that. Unfortunately since I need to import NativeStringIO it isn't possible for me to keep StringIO.StringIO which is why I did it the way I did. Is there a better way?

Thanks for your patience and help.

Also, is there a way I should go about submitting additional patches when the first ones are inadequate? The way I have been doing it seems suboptimal.

[Josh Schneier]

First of all, let me say thanks for your patience and help. However, I'm confused, can you explain why?

1. I looked through twisted.internet.utils and didn't find any hint of reraise why would I want to still import it in general?

2. I removed the _utilspy3 ones and didn't put the utils and test_iutils in. Per your last comment I shouldn't try and port the files as a whole, so I removed them from the ported list.

[Tom Prince]

1. I looked through twisted.internet.utils and didn't find any hint of reraise why would I want to still import it in general?

https://twistedmatrix.com/trac/browser/branches/internet-utils-mergs-6240/twisted/internet/utils.py?rev=#L203

2. I removed the _utilspy3 ones and didn't put the utils and test_iutils in. Per your last comment I shouldn't try and port the files as a whole, so I removed them from the ported list.

Well, some of it was ported, so the modules should get installed, and the tests run, which is the main purpose of admin/_twistedpython3.py. The dels at the end of those modules will remove any trace of the unported parts, leaving just the ported parts useable and testable on python 3.

[Tom Prince]

3. I tried to do just that. Unfortunately since I need to import NativeStringIO it isn't possible for me to keep StringIO.StringIO which is why I did it the way I did. Is there a better way?

Since that code never actually gets run (just parsed), it doesn't in fact need NativeStringIO. It will when that code gets ported, and the change you did is probably the right thing to do there. (What happens is the function/classes get defined, then get deleted before they are ever called.)

Also, is there a way I should go about submitting additional patches when the first ones are inadequate? The way I have been doing it seems suboptimal.

Suboptimal how? If your patches have been applied to a branch, you should base future patches off the tip of the branch. That is often the case, but I didn't do that here. (If you have further patches for this ticket, they should be against the branch).

[therve]

1. I don't think you need to do "del ProcessUtilsTests", the tests will skip properly.

Looks good, please merge.

[Tom Prince]

(In [37873]) Branching to internet-utils-mergs-6240-2.

[Tom Prince]

1. I don't think you need to do "del ProcessUtilsTests", the tests will skip properly.

The code they are testing hasn't been ported yet. Deleting the tests is the idiom that has been used to indicate that. In any case, the process support in the reactor will get ported to python3 before the process support in t.i.utils, which will cause these tests to start failing.

[Tom Prince]

(In [37876]) Merge internet-utils-mergs-6240-2: Merge twisted/internet/_utilspy3.py into twisted/internet/utils.py.

Author: genericnick, tom.prince Reviewers: tom.prince, therve Fixes: #6240