Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(845)

Issue 842043: PEP 3147 implementation (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years, 5 months ago by barry
Modified:
6 years, 11 months ago
Base URL:
http://svn.python.org/view/*checkout*/python/branches/py3k/
Visibility:
Public.

Patch Set 1 #

Total comments: 72

Patch Set 2 : Updated PEP 3147 implementation #

Total comments: 52

Patch Set 3 : Second review updates #

Patch Set 4 : Added support for module.__cached__ and some windows fixes #

Total comments: 48

Patch Set 5 : Review updates and other changes #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1163 lines, -270 lines) Patch
M .bzrignore View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M .hgignore View 1 chunk +1 line, -0 lines 0 comments Download
M Doc/c-api/import.rst View 2 chunks +17 lines, -0 lines 0 comments Download
M Doc/library/compileall.rst View 2 chunks +9 lines, -3 lines 0 comments Download
M Doc/library/imp.rst View 3 4 1 chunk +35 lines, -2 lines 0 comments Download
M Doc/library/py_compile.rst View 3 4 1 chunk +7 lines, -3 lines 0 comments Download
M Doc/library/runpy.rst View 2 chunks +4 lines, -1 line 0 comments Download
M Include/import.h View 4 1 chunk +3 lines, -0 lines 0 comments Download
M Lib/compileall.py View 11 chunks +48 lines, -26 lines 0 comments Download
M Lib/importlib/_bootstrap.py View 1 2 3 4 2 chunks +20 lines, -1 line 0 comments Download
M Lib/importlib/test/__main__.py View 1 2 3 4 1 chunk +6 lines, -1 line 0 comments Download
M Lib/importlib/test/source/test_file_loader.py View 1 2 3 4 4 chunks +5 lines, -4 lines 0 comments Download
M Lib/importlib/test/source/test_finder.py View 1 2 3 4 3 chunks +12 lines, -1 line 0 comments Download
M Lib/importlib/test/source/test_source_encoding.py View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M Lib/importlib/test/source/util.py View 1 2 3 4 2 chunks +11 lines, -8 lines 0 comments Download
M Lib/importlib/util.py View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M Lib/inspect.py View 1 chunk +1 line, -0 lines 0 comments Download
M Lib/py_compile.py View 1 2 3 4 7 chunks +27 lines, -21 lines 0 comments Download
M Lib/pydoc.py View 1 chunk +2 lines, -1 line 0 comments Download
M Lib/runpy.py View 2 chunks +2 lines, -0 lines 0 comments Download
M Lib/test/script_helper.py View 1 2 3 4 3 chunks +12 lines, -11 lines 0 comments Download
M Lib/test/support.py View 1 2 3 4 4 chunks +64 lines, -26 lines 0 comments Download
M Lib/test/test_cmd_line_script.py View 1 2 3 4 6 chunks +18 lines, -12 lines 0 comments Download
M Lib/test/test_compileall.py View 1 2 3 4 3 chunks +69 lines, -10 lines 0 comments Download
M Lib/test/test_frozen.py View 4 3 chunks +5 lines, -5 lines 0 comments Download
M Lib/test/test_imp.py View 1 2 3 4 3 chunks +125 lines, -2 lines 0 comments Download
M Lib/test/test_import.py View 1 2 3 4 11 chunks +176 lines, -30 lines 0 comments Download
M Lib/test/test_pkg.py View 4 3 chunks +10 lines, -10 lines 0 comments Download
M Lib/test/test_pkgimport.py View 1 2 3 4 2 chunks +14 lines, -12 lines 0 comments Download
M Lib/test/test_pydoc.py View 1 2 3 4 5 chunks +7 lines, -9 lines 0 comments Download
M Lib/test/test_runpy.py View 1 2 3 4 10 chunks +17 lines, -8 lines 0 comments Download
M Lib/test/test_zipfile.py View 2 3 4 2 chunks +8 lines, -1 line 0 comments Download
M Lib/test/test_zipimport.py View 1 2 3 4 6 chunks +19 lines, -20 lines 0 comments Download
M Lib/zipfile.py View 2 3 4 2 chunks +40 lines, -13 lines 0 comments Download
M Makefile.pre.in View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M Python/import.c View 1 2 3 4 20 chunks +363 lines, -28 lines 0 comments Download
M Python/pythonrun.c View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 12
Antoine Pitrou
Hi Barry, here are some comments. I should stress than I have not tried applying ...
7 years, 5 months ago (2010-04-04 15:55:30 UTC) #1
barry
Thanks for the great review Antoine. I will have a new patchset to upload based ...
7 years, 5 months ago (2010-04-06 23:05:09 UTC) #2
Benjamin
Looks pretty good to me. I basically only picked on style things. I think it ...
7 years, 5 months ago (2010-04-07 02:50:04 UTC) #3
brett.cannon
Just reviewed the importlib stuff. http://codereview.appspot.com/842043/diff/13001/14009 File Lib/importlib/_bootstrap.py (right): http://codereview.appspot.com/842043/diff/13001/14009#newcode491 Lib/importlib/_bootstrap.py:491: def _find_path(self, ext_type): The ...
7 years, 5 months ago (2010-04-07 20:19:02 UTC) #4
barry
Thanks for the review guys. I'll be uploading a new patch set soon. http://codereview.appspot.com/842043/diff/13001/14009 File ...
7 years, 5 months ago (2010-04-08 12:53:47 UTC) #5
benjamin_python.org
2010/4/8 <warsaw@gmail.com>: > http://codereview.appspot.com/842043/diff/13001/14002#newcode3371 > Python/import.c:3371: static char *kwlist[] = {"path", "force__debug__", > NULL}; > ...
7 years, 5 months ago (2010-04-08 20:54:53 UTC) #6
barry
On 2010/04/08 20:54:53, benjamin_python.org wrote: > 2010/4/8 <warsaw@gmail.com>: > > http://codereview.appspot.com/842043/diff/13001/14002#newcode3371 > > Python/import.c:3371: static ...
7 years, 5 months ago (2010-04-09 15:09:29 UTC) #7
Antoine Pitrou
Some further comments. http://codereview.appspot.com/842043/diff/32001/33003 File Include/import.h (right): http://codereview.appspot.com/842043/diff/32001/33003#newcode14 Include/import.h:14: PyAPI_FUNC(PyObject *) PyImport_ExecCodeModuleExEx( How about `PyImport_ExecCodeModuleWithPathname`? ...
7 years, 5 months ago (2010-04-09 23:39:32 UTC) #8
Georg
http://codereview.appspot.com/842043/diff/32001/33001 File .bzrignore (right): http://codereview.appspot.com/842043/diff/32001/33001#newcode35 .bzrignore:35: __pycache__ Should also add that to .hgignore :) http://codereview.appspot.com/842043/diff/32001/33006 ...
7 years, 5 months ago (2010-04-10 19:07:56 UTC) #9
barry
Thanks for the review. Will be pushing a new patch set momentarily that also includes ...
7 years, 5 months ago (2010-04-12 21:59:05 UTC) #10
pjenvey
Barry, just one small thing: __pycached__ is probably just like __file__ in that it could ...
7 years, 5 months ago (2010-04-13 22:47:41 UTC) #11
barry
7 years, 5 months ago (2010-04-14 23:56:57 UTC) #12
On 2010/04/13 22:47:41, pjenvey wrote:
> Barry, just one small thing:
> 
> __pycached__ is probably just like __file__ in that it could end up being a
> relative path before site has been fully imported. So I'm guessing site needs
to
> abspath() the initial set of sys.modules' __pycached__ values like it already
> does for their __file__s
> 
> You should be able to see a case where they would be relative with an svn
> checkout, via PYTHONPATH=Lib/ ./python

Note that test_abs__file__() in test_site.py is pretty crappy, and broken.  I
have a much better test for this, and added a test to ensure abspath'd
__cached__.

Thanks for pointing this one out.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 80a51fa-tainted