msg192237 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2013-07-03 13:11 |
http://hg.python.org/cpython/file/8e838598eed1/Lib/importlib/_bootstrap.py#l455 : source_stats does not exist and instead should be source_path. |
|
|
msg192300 - (view) |
Author: Madison May (madison.may) * |
Date: 2013-07-04 17:19 |
Here's a 5 character patch for the sake of completeness. |
|
|
msg192304 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2013-07-04 19:02 |
Any chance you want to write some tests for the function? =) |
|
|
msg192337 - (view) |
Author: Madison May (madison.may) * |
Date: 2013-07-05 17:12 |
I'd be glad to -- I'll get right to work =). On a related note, rpartition is also misspelled in _get_sourcefile() on line 446. |
|
|
msg192338 - (view) |
Author: Madison May (madison.may) * |
Date: 2013-07-05 17:26 |
You might have to bear with me -- I'm a bit new to this. What's the protocol for functions like _get_sourcepath() that require support files for testing? I'll should probably have a couple .pyc's, .pyo's and .py files to use for testing purposes. Additionally, would you recommend writing tests for _get_sourcepath() in test_imp.py, or would another location prove better in your opinion? |
|
|
msg192339 - (view) |
Author: Madison May (madison.may) * |
Date: 2013-07-05 17:36 |
Yet another _get_sourcefile() related bug to report. Line 453 should be: source_path = bytcode_path[:-1] instead of source_path = bytcode_path[-1:] |
|
|
msg192340 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2013-07-05 17:48 |
Wow, I really botched that function. And no one seems to really use it to notice the errors (or at least the branches in the function where the errors exist are never reached). =) So the tests should go in test_importlib or test_import (I'm leaning towards the latter since this function is for the C API). As for the files, I wouldn't create any and instead mock out importlib._bootstrap._path_isfile() to return the value you want. And one last thing, Madison, is to do the patch against Python 3.3 if you can. This obviously needs to be backported so it's best to start there and then work forward. |
|
|
msg192363 - (view) |
Author: Madison May (madison.may) * |
Date: 2013-07-05 21:52 |
Here's a preliminary attempt at a patch and a small set of test cases for _get_sourcefile. I fixed one more spelling error in _get_sourcefile (bytcode -> bytecode) and tweaked a bit of logic. Instead of: extension.lower()[-3:-1] != '.py' I think we needed extension.lower()[-3:-1] != 'py' You must have been having a rough day, Brett =). Anyhow, when you get a chance, take a look at this first attempt and let me know what you'd do differently. Thanks, Madison |
|
|
msg192472 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2013-07-06 17:29 |
When I wrote that code I was struggling to untangle the C code and it was driving me up the wall because it was code unique to the C API and was just ever so slightly different. Anyway, the patch looks great! When I have time I will commit it to 3.3 and 3.4. |
|
|
msg192482 - (view) |
Author: Madison May (madison.may) * |
Date: 2013-07-06 19:31 |
I can imagine that would be incredibly frustrating -- it would drive me up the wall as well. Anyhow, glad to hear things looked good. I really appreciate your help guiding me through this one. |
|
|
msg192490 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2013-07-06 22:05 |
New changeset b8028f74bac4 by Brett Cannon in branch '3.3': Issue #18351: Fix various issues with http://hg.python.org/cpython/rev/b8028f74bac4 New changeset e80634ad5a0e by Brett Cannon in branch 'default': merge for issue #18351. http://hg.python.org/cpython/rev/e80634ad5a0e |
|
|
msg192493 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2013-07-06 22:15 |
Thanks for the patch, Madison! Added you to the ACKS file. |
|
|