| 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) *  |
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) * |
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) * |
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) * |
Date: 2008-07-15 18:52 |
| Today or tomorrow. |
|
|
| msg69731 - (view) |
Author: Barry A. Warsaw (barry) * |
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) * |
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) * |
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) * |
Date: 2008-07-16 05:13 |
| r65002 has the patch. Thanks, Nick! |
|
|