classification
Title: Proposal for fix_urllib
Type: Stage:
Components: 2to3 (2.x to 3.x conversion tool) Versions:
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: collinwinter Nosy List: barry, benjamin.peterson, brett.cannon, collinwinter, nedds, orsenthil
Priority: deferred blocker Keywords: patch

Created on 2008-07-07 20:36 by nedds, last changed 2008-07-16 05:13 by brett.cannon. This issue is now closed.

Files
File name Uploaded Description Edit
fix_urllib.diff nedds, 2008-07-07 20:36 Diff for fix_urllib
fix_urllib.diff nedds, 2008-07-07 21:12 Diff for updated fix_urllib
fix_urllib.diff nedds, 2008-07-16 03:58 Diff for fix_urllib and test suite
Messages (13)
msg69395 - (view) Author: Nick Edds (nedds) Date: 2008-07-07 20:36
Here is my proposed fix_urllib. The transform function is massive
because there are a lot of cases, so maybe I should break it into
separate functions for each case, and it could maybe do with some more
documentation as well. I also use FromImport from fixer_util.py even
though it warns against doing so for dotted imports because it appears
to work. The temp.py file is a dummy python file that you can test it on
to get an idea of what it does. I think it's got all the desired
functionality, but I'm not an expert on the urllib changes so let me
know if I missed anything. I haven't written tests for it yet, but
assuming it looks reasonable, I can do that later today or tomorrow.
msg69399 - (view) Author: Nick Edds (nedds) Date: 2008-07-07 21:12
I broke up transform into the logical pieces of it, so I think now the
code is a little bit more clear.
msg69513 - (view) Author: Collin Winter (collinwinter) * (Python committer) Date: 2008-07-10 17:42
- You should add tests to test_fixers to ensure that this does what you
expect. This will make it much easier for others to modify this fixer
later. You can probably just reuse the tests Brett initially added.

- There's a better data structure for MAPPING: rather than using

MAPPING = {module: [(new_module, [member, member, ..., member])]}

you should probably use

MAPPING = {module: {new_module: [member, member, ..., member]}}

The list-of-2-tuples was a bad idea on my part that didn't scale.

- The "cannot handle star imports" warning isn't strictly correct: star
imports for robotparser and urlparse can both be handled correctly,
since they're just being renamed.

- urlparse and robotparser don't seem to belong to this more specialized
fixer: they can go in fix_imports, since they aren't being broken across
multiple modules.
msg69525 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2008-07-11 00:08
What Collin said. =)

I will put robotparser and urlparse into fix_imports myself and continue
to update the docs in 2.x for urllib(2).

Thanks to the both of you for helping with this! It's going to be great
once this fixer is ready to go as it will put PEP 3108 REALLY close to
being finished!
msg69704 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2008-07-15 18:35
Nick, are you going to be able to get this cleaned up today for the beta
release tomorrow?
msg69706 - (view) Author: Nick Edds (nedds) Date: 2008-07-15 18:45
I should be able to. What time would you need it by?
msg69708 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-07-15 18:52
Today or tomorrow.
msg69731 - (view) Author: Barry A. Warsaw (barry) * (Python committer) Date: 2008-07-15 23:44
I would really like to get this in by beta2, but it's not quite enough
to hold up the release.  Please try to get this cleaned up and committed
by July 16 for beta 2.
msg69733 - (view) Author: Nick Edds (nedds) Date: 2008-07-16 00:10
I've got it finished, I just need to write some tests for it. It will
all be done later tonight.
msg69735 - (view) Author: Barry A. Warsaw (barry) * (Python committer) Date: 2008-07-16 00:22
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Jul 15, 2008, at 8:10 PM, Nick Edds wrote:

> Nick Edds <nedds@uchicago.edu> added the comment:
>
> I've got it finished, I just need to write some tests for it. It will
> all be done later tonight.

Great!

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Darwin)

iQCVAwUBSH0/RHEjvBPtnXfVAQIdvAP/Sxz4XayNMlBWIicFVmmQ/C85Fujsb7wN
tqznDWfM7rqCiChkSe2dMPoMTQsOntCIEze+d1Usc5SqqVQk45dSX/ARs2qVtI6g
siVDSAxt2e1DWVCpko7hDySfP70e+1Id+Uv9obrHDJ6p6l7tx2hcZeS24NtQ+7aX
vrCfAtAJZxk=
=vtPk
-----END PGP SIGNATURE-----
msg69757 - (view) Author: Nick Edds (nedds) Date: 2008-07-16 03:58
Here is a working version with tests. I believe it has all the desired
functionality, but correct me if I'm wrong Brett. Also, I did not model
MAPPING as suggested by Collin because order was important in generating
tests, so I didn't think using a dictionary would be appropriate. As for
star imports, right now they aren't supported, although I don't quite
understand why they couldn't be. Also, if this looks good, could
somebody commit it for me. I don't think I have write privileges. Sorry
this took me until so late tonight to submit, I hadn't realized the high
priority it had until this morning.
msg69763 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2008-07-16 04:39
I got your patch and it looks good overall. I made some style changes
and upped the code reuse in some places. I am running the test suite now
before I commit.
msg69765 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2008-07-16 05:13
r65002 has the patch. Thanks, Nick!
History
Date User Action Args
2008-07-16 05:13:47brett.cannonsetstatus: pending -> closed
resolution: accepted
messages: + msg69765
2008-07-16 04:39:41brett.cannonsetstatus: open -> pending
messages: + msg69763
2008-07-16 03:58:09neddssetfiles: + fix_urllib.diff
messages: + msg69757
2008-07-16 00:22:30barrysetmessages: + msg69735
2008-07-16 00:10:20neddssetmessages: + msg69733
2008-07-15 23:44:36barrysetpriority: release blocker -> deferred blocker
nosy: + barry
messages: + msg69731
2008-07-15 18:52:21benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg69708
2008-07-15 18:45:53neddssetmessages: + msg69706
2008-07-15 18:35:27brett.cannonsetmessages: + msg69704
2008-07-15 18:33:36brett.cannonsetpriority: release blocker
2008-07-13 22:42:59brett.cannonlinkissue2885 dependencies
2008-07-11 00:08:02brett.cannonsetmessages: + msg69525
2008-07-10 17:42:34collinwintersetmessages: + msg69513
2008-07-09 03:07:04orsenthilsetnosy: + orsenthil
2008-07-07 21:12:05neddssetfiles: + fix_urllib.diff
messages: + msg69399
2008-07-07 20:36:53neddscreate