Repository navigation
Proposal for fix_urllib #47566
Description
| BPO | 3316 |
|---|---|
| Nosy | @warsaw, @brettcannon, @orsenthil, @benjaminp |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = <Date 2008-07-16.05:13:47.214>
created_at = <Date 2008-07-07.20:36:53.430>
labels = ['deferred-blocker', 'expert-2to3']
title = 'Proposal for fix_urllib'
updated_at = <Date 2008-07-16.05:13:47.213>
user = 'https://bugs.python.org/nedds'bugs.python.org fields:
activity = <Date 2008-07-16.05:13:47.213>
actor = 'brett.cannon'
assignee = 'collinwinter'
closed = True
closed_date = <Date 2008-07-16.05:13:47.214>
closer = 'brett.cannon'
components = ['2to3 (2.x to 3.x conversion tool)']
creation = <Date 2008-07-07.20:36:53.430>
creator = 'nedds'
dependencies = []
files = ['10845', '10846', '10903']
hgrepos = []
issue_num = 3316
keywords = ['patch']
message_count = 13.0
messages = ['69395', '69399', '69513', '69525', '69704', '69706', '69708', '69731', '69733', '69735', '69757', '69763', '69765']
nosy_count = 6.0
nosy_names = ['barry', 'brett.cannon', 'collinwinter', 'orsenthil', 'benjamin.peterson', 'nedds']
pr_nums = []
priority = 'deferred blocker'
resolution = 'accepted'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue3316'
versions = []
Activity
nedds commentedon Jul 7, 2008
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.
nedds commentedon Jul 7, 2008
I broke up transform into the logical pieces of it, so I think now the
code is a little bit more clear.
collinwinter commentedon Jul 10, 2008
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
you should probably use
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.
brettcannon commentedon Jul 11, 2008
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!
brettcannon commentedon Jul 15, 2008
Nick, are you going to be able to get this cleaned up today for the beta
release tomorrow?
nedds commentedon Jul 15, 2008
I should be able to. What time would you need it by?
benjaminp commentedon Jul 15, 2008
Today or tomorrow.
warsaw commentedon Jul 15, 2008
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.
nedds commentedon Jul 16, 2008
I've got it finished, I just need to write some tests for it. It will
all be done later tonight.
warsaw commentedon Jul 16, 2008
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On Jul 15, 2008, at 8:10 PM, Nick Edds wrote:
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-----
nedds commentedon Jul 16, 2008
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.
brettcannon commentedon Jul 16, 2008
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.
brettcannon commentedon Jul 16, 2008
r65002 has the patch. Thanks, Nick!