Issue 18351: Incorrect variable name in importlib._bootstrap._get_sourcefile (original) (raw)

Created on 2013-07-03 13:11 by brett.cannon, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
Issue18351.patch madison.may,2013-07-04 17:19 review
Issue18351_Python3-3.patch madison.may,2013-07-05 21:52 Additional bug fixes + a few test cases in test_import.py review
Messages (12)
msg192237 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) (Python triager) 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) * (Python committer) Date: 2013-07-06 22:15
Thanks for the patch, Madison! Added you to the ACKS file.
History
Date User Action Args
2022-04-11 14:57:47 admin set github: 62551
2013-07-06 22:15:42 brett.cannon set status: open -> closedresolution: fixedmessages: + stage: commit review -> resolved
2013-07-06 22:05:12 python-dev set nosy: + python-devmessages: +
2013-07-06 19:31:01 madison.may set messages: +
2013-07-06 17:30:29 brett.cannon set stage: test needed -> commit review
2013-07-06 17:29:20 brett.cannon set messages: +
2013-07-05 21:52:46 madison.may set files: + Issue18351_Python3-3.patchmessages: +
2013-07-05 17:48:46 brett.cannon set messages: +
2013-07-05 17:36:49 madison.may set messages: +
2013-07-05 17:26:12 madison.may set messages: +
2013-07-05 17:12:02 madison.may set messages: +
2013-07-04 19:02:42 brett.cannon set keywords: + easymessages: +
2013-07-04 17:19:43 madison.may set files: + Issue18351.patchnosy: + madison.maymessages: + keywords: + patch
2013-07-03 13:11:12 brett.cannon create