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

#23731: Implement PEP 488

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 5 months ago by brett
Modified:
4 years, 5 months ago
Reviewers:
p.f.moore, nad, ericsnowcurrently
CC:
barry, brett.cannon, gregory.p.smith, pmoore, tim.golden, devnull_psf.upfronthosting.co.za, eric.snow, Zach Ware, steve.dower
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Total comments: 51

Patch Set 3 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/c-api/import.rst View 1 2 1 chunk +3 lines, -3 lines 0 comments Download
Doc/distutils/apiref.rst View 1 2 2 chunks +7 lines, -4 lines 0 comments Download
Doc/distutils/introduction.rst View 1 2 2 chunks +2 lines, -4 lines 0 comments Download
Doc/library/compileall.rst View 1 2 4 chunks +16 lines, -0 lines 0 comments Download
Doc/library/importlib.rst View 1 2 4 chunks +34 lines, -12 lines 0 comments Download
Doc/library/imp.rst View 1 2 2 chunks +6 lines, -5 lines 0 comments Download
Doc/library/py_compile.rst View 1 2 3 chunks +4 lines, -5 lines 0 comments Download
Doc/library/sys.rst View 1 2 2 chunks +1 line, -2 lines 0 comments Download
Doc/library/tracemalloc.rst View 1 2 3 chunks +5 lines, -2 lines 0 comments Download
Doc/library/zipfile.rst View 1 2 2 chunks +1 line, -3 lines 0 comments Download
Doc/library/zipimport.rst View 1 2 2 chunks +2 lines, -3 lines 0 comments Download
Doc/reference/import.rst View 1 2 1 chunk +1 line, -1 line 0 comments Download
Doc/tutorial/modules.rst View 1 2 2 chunks +5 lines, -6 lines 0 comments Download
Doc/using/cmdline.rst View 1 2 1 chunk +1 line, -1 line 0 comments Download
Doc/using/windows.rst View 1 2 3 chunks +2 lines, -4 lines 0 comments Download
Doc/whatsnew/3.5.rst View 1 2 5 chunks +34 lines, -1 line 0 comments Download
Lib/compileall.py View 1 2 2 chunks +4 lines, -3 lines 0 comments Download
Lib/distutils/command/build_py.py View 1 2 1 chunk +2 lines, -2 lines 0 comments Download
Lib/distutils/command/install_lib.py View 1 2 2 chunks +8 lines, -8 lines 0 comments Download
Lib/distutils/tests/test_build_py.py View 1 2 1 chunk +2 lines, -2 lines 0 comments Download
Lib/distutils/tests/test_install_lib.py View 1 2 2 chunks +6 lines, -7 lines 0 comments Download
Lib/distutils/util.py View 1 2 2 chunks +5 lines, -4 lines 0 comments Download
Lib/doctest.py View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
Lib/importlib/_bootstrap.py View 1 2 3 chunks +54 lines, -30 lines 0 comments Download
Lib/imp.py View 1 2 2 chunks +10 lines, -9 lines 0 comments Download
Lib/modulefinder.py View 1 2 1 chunk +1 line, -1 line 0 comments Download
Lib/msilib/__init__.py View 1 2 2 chunks +3 lines, -4 lines 0 comments Download
Lib/py_compile.py View 1 2 4 chunks +6 lines, -5 lines 0 comments Download
Lib/pydoc.py View 1 2 1 chunk +1 line, -1 line 0 comments Download
Lib/test/support/__init__.py View 1 2 1 chunk +8 lines, -12 lines 0 comments Download
Lib/test/test_compileall.py View 1 2 3 chunks +10 lines, -11 lines 0 comments Download
Lib/test/test_compile.py View 1 2 1 chunk +1 line, -1 line 0 comments Download
Lib/test/test_import/__init__.py View 1 2 13 chunks +16 lines, -21 lines 0 comments Download
Lib/test/test_importlib/test_util.py View 1 2 5 chunks +96 lines, -18 lines 0 comments Download
Lib/test/test_imp.py View 1 2 4 chunks +1 line, -114 lines 0 comments Download
Lib/test/test_py_compile.py View 1 2 1 chunk +4 lines, -0 lines 0 comments Download
Lib/test/test_runpy.py View 1 2 1 chunk +1 line, -1 line 0 comments Download
Lib/test/test_tracemalloc.py View 1 2 3 chunks +1 line, -5 lines 0 comments Download
Lib/test/test_trace.py View 1 2 1 chunk +2 lines, -2 lines 0 comments Download
Lib/test/test_zipfile.py View 1 2 4 chunks +4 lines, -7 lines 0 comments Download
Lib/test/test_zipimport.py View 1 2 1 chunk +1 line, -1 line 0 comments Download
Lib/tkinter/__init__.py View 1 2 1 chunk +1 line, -1 line 0 comments Download
Lib/tkinter/test/runtktests.py View 1 2 1 chunk +1 line, -1 line 0 comments Download
Lib/tracemalloc.py View 1 2 1 chunk +1 line, -1 line 0 comments Download
Lib/trace.py View 1 2 1 chunk +1 line, -1 line 0 comments Download
Lib/unittest/loader.py View 1 2 1 chunk +2 lines, -2 lines 0 comments Download
Lib/warnings.py View 1 2 1 chunk +1 line, -1 line 0 comments Download
Lib/zipfile.py View 1 2 2 chunks +35 lines, -22 lines 0 comments Download
Makefile.pre.in View 1 2 2 chunks +10 lines, -1 line 0 comments Download
Misc/python.man View 1 2 4 chunks +11 lines, -14 lines 0 comments Download
Modules/getpath.c View 1 2 2 chunks +2 lines, -3 lines 0 comments Download
Modules/zipimport.c View 1 2 3 chunks +2 lines, -15 lines 0 comments Download
Python/importlib.h View 1 2 2 chunks +4287 lines, -4226 lines 0 comments Download
Python/pythonrun.c View 1 2 2 chunks +1 line, -4 lines 0 comments Download
Python/_warnings.c View 1 2 1 chunk +2 lines, -3 lines 0 comments Download

Messages

Total messages: 5
pmoore
http://bugs.python.org/review/23731/diff/14413/Doc/library/compileall.rst File Doc/library/compileall.rst (right): http://bugs.python.org/review/23731/diff/14413/Doc/library/compileall.rst#newcode159 Doc/library/compileall.rst:159: no matter what the value of *optimize* is. Is ...
4 years, 5 months ago #1
nad_acm.org
Please don't make this change to build-installer.py. I try to keep the source identical across ...
4 years, 5 months ago #2
brett.cannon
I just wanted to answer some of Paul's questions now rather than wait until I ...
4 years, 5 months ago #3
eric.snow
Reading through this change makes me happy. :) I have a few minor comments here ...
4 years, 5 months ago #4
brett.cannon
4 years, 5 months ago #5
PTAL; I think I have addressed everyone's comments.

http://bugs.python.org/review/23731/diff/14413/Doc/library/importlib.rst
File Doc/library/importlib.rst (right):

http://bugs.python.org/review/23731/diff/14413/Doc/library/importlib.rst#newc...
Doc/library/importlib.rst:730: modules (including the leading dot).
On 2015/04/02 07:19:17, eric.snow wrote:
> Nice addition about the leading dot. :)

ACK

http://bugs.python.org/review/23731/diff/14413/Doc/library/sys.rst
File Doc/library/sys.rst (left):

http://bugs.python.org/review/23731/diff/14413/Doc/library/sys.rst#oldcode1234
Doc/library/sys.rst:1234: 
On 2015/04/02 07:19:17, eric.snow wrote:
> Aren't such blank lines there for the sake of some editors?  emacs?

There still is a blank line; there just happened to be 2 newlines at the end of
the file instead of just one (otherwise there would be a note about no newline
at the end of the file).

http://bugs.python.org/review/23731/diff/14413/Doc/whatsnew/3.5.rst
File Doc/whatsnew/3.5.rst (right):

http://bugs.python.org/review/23731/diff/14413/Doc/whatsnew/3.5.rst#newcode735
Doc/whatsnew/3.5.rst:735: *debug_override* parameter of the function is now
deprecated.
On 2015/04/02 07:19:17, eric.snow wrote:
> Does there need to be any explicit reminder about compatibility between
> byte-compiled modules from different versions.  The PEP talked about this
> specifically.
> 
> What about .pyc-distributed-with-.py?  This could have an impact on
third-party
> tools, no?

Done.

http://bugs.python.org/review/23731/diff/14413/Lib/imp.py
File Lib/imp.py (right):

http://bugs.python.org/review/23731/diff/14413/Lib/imp.py#newcode85
Lib/imp.py:85: with warnings.catch_warnings():
On 2015/04/02 07:19:17, eric.snow wrote:
> nice

ACK

http://bugs.python.org/review/23731/diff/14413/Lib/importlib/_bootstrap.py
File Lib/importlib/_bootstrap.py (right):

http://bugs.python.org/review/23731/diff/14413/Lib/importlib/_bootstrap.py#ne...
Lib/importlib/_bootstrap.py:461: if optimization is not None:
On 2015/04/02 07:19:17, eric.snow wrote:
> nice

ACK

http://bugs.python.org/review/23731/diff/14413/Lib/test/support/__init__.py
File Lib/test/support/__init__.py (right):

http://bugs.python.org/review/23731/diff/14413/Lib/test/support/__init__.py#n...
Lib/test/support/__init__.py:378: """Move a PEP 3147 pyc file to its legacy pyc
location.
On 2015/04/01 21:22:59, pmoore wrote:
> PEP 3147/488? Really minor point...

Done.

http://bugs.python.org/review/23731/diff/14413/Lib/test/support/__init__.py#n...
Lib/test/support/__init__.py:400: # combinations of PEP 3147 and legacy pyc
files.
On 2015/04/01 21:22:59, pmoore wrote:
> 3747/488 again...

Done.

http://bugs.python.org/review/23731/diff/14413/Lib/test/support/__init__.py#n...
Lib/test/support/__init__.py:402: for opt in ('', 1, 2):
On 2015/04/02 07:19:17, eric.snow wrote:
> Does there need to be an explicit list somewhere of compiler-supported
> optimization levels (e.g. sys.implementation.opt_levels)?  The levels here are
> CPython-specific and could change later as well.

There could be, but I would rather not introduce it at this exact moment. I have
opened http://bugs.python.org/issue23892 for the idea.

http://bugs.python.org/review/23731/diff/14413/Lib/test/test_compileall.py
File Lib/test/test_compileall.py (right):

http://bugs.python.org/review/23731/diff/14413/Lib/test/test_compileall.py#ne...
Lib/test/test_compileall.py:243: ('optimize', 'opt-1.pyc', ['-O']),
On 2015/04/02 07:19:17, eric.snow wrote:
> should each of these suffixes include the leading '.'?

No. Look at the line above and notice how `pyc` has no leading dot. You can also
see how in line 251 the string format already includes the leading dot.

http://bugs.python.org/review/23731/diff/14413/Lib/test/test_compileall.py#ne...
Lib/test/test_compileall.py:243: ('optimize', 'opt-1.pyc', ['-O']),
On 2015/04/02 07:19:17, eric.snow wrote:
> should each of these suffixes include the leading '.'?

No; see line 251 as well as the fact that `pyc` on the line above has no leading
dot.

http://bugs.python.org/review/23731/diff/14413/Lib/test/test_imp.py
File Lib/test/test_imp.py (left):

http://bugs.python.org/review/23731/diff/14413/Lib/test/test_imp.py#oldcode114
Lib/test/test_imp.py:114: support.unlink(temp_mod_name + '.pyo')
On 2015/04/02 07:19:17, eric.snow wrote:
> ah, this is just for .pyc-next-to-.py, right?  I was going to ask about the
> "opt-" tag, but that doesn't apply here, right?

It's only cleaning up source and legacy .pyc file locations, so the .pyo is now
redundant.

http://bugs.python.org/review/23731/diff/14413/Lib/test/test_import/__init__.py
File Lib/test/test_import/__init__.py (left):

http://bugs.python.org/review/23731/diff/14413/Lib/test/test_import/__init__....
Lib/test/test_import/__init__.py:35: name + ".pyo",
On 2015/04/02 07:19:17, eric.snow wrote:
> This only matters for the .pyc-next-to-.py case so there's no need to worry
> about "opt-" tags, right?

Yes

http://bugs.python.org/review/23731/diff/14413/Lib/test/test_import/__init__.py
File Lib/test/test_import/__init__.py (right):

http://bugs.python.org/review/23731/diff/14413/Lib/test/test_import/__init__....
Lib/test/test_import/__init__.py:672: '__pycache__', '{}.{}.pyc'.format(TESTFN,
self.tag))))
On 2015/04/02 07:19:17, eric.snow wrote:
> Shouldn't this get the same treatment as in test_import_pyc_path.  Then we
could
> get rid of self.tag.

Done.

http://bugs.python.org/review/23731/diff/14413/Lib/test/test_importlib/test_u...
File Lib/test/test_importlib/test_util.py (right):

http://bugs.python.org/review/23731/diff/14413/Lib/test/test_importlib/test_u...
Lib/test/test_importlib/test_util.py:587: with warnings.catch_warnings():
On 2015/04/02 07:19:17, eric.snow wrote:
> nice

ACK

http://bugs.python.org/review/23731/diff/14413/Lib/test/test_importlib/test_u...
Lib/test/test_importlib/test_util.py:717: '__pycache__/foo.cpython-32.foo.pyc')
On 2015/04/02 07:19:17, eric.snow wrote:
> What about a too_many_dots test?  "foo.cpython-32.opt-1.foo.pyc"

Done.

http://bugs.python.org/review/23731/diff/14413/Lib/test/test_zipfile.py
File Lib/test/test_zipfile.py (left):

http://bugs.python.org/review/23731/diff/14413/Lib/test/test_zipfile.py#oldco...
Lib/test/test_zipfile.py:692: if fn.endswith('.pyc') or fn.endswith('.pyo'):
On 2015/04/02 07:19:17, eric.snow wrote:
> how funny. :)

ACK

http://bugs.python.org/review/23731/diff/14413/Lib/zipfile.py
File Lib/zipfile.py (right):

http://bugs.python.org/review/23731/diff/14413/Lib/zipfile.py#newcode1827
Lib/zipfile.py:1827: pycache_opt0 = importlib.util.cache_from_source(file_py,
optimization='')
On 2015/04/02 07:19:17, eric.snow wrote:
> again, this makes we wonder if something like sys.implementation.opt_levels
> might be worth adding.

See other comment and http://bugs.python.org/issue23892

http://bugs.python.org/review/23731/diff/14413/Lib/zipfile.py#newcode1872
Lib/zipfile.py:1872: fname = pycache_opt1 if self._optimize == 1 else
pycache_opt2
On 2015/04/02 07:19:17, eric.snow wrote:
> fragile special casing alert!

Done.

http://bugs.python.org/review/23731/diff/14413/Makefile.pre.in
File Makefile.pre.in (right):

http://bugs.python.org/review/23731/diff/14413/Makefile.pre.in#newcode1266
Makefile.pre.in:1266: -PYTHONPATH=$(DESTDIR)$(LIBDEST) $(RUNSHARED) \
On 2015/04/02 07:19:17, eric.snow wrote:
> What's do the changes in this file do?  How do they relate to the .pyc
changes?

Compiles stdlib with -OO now on top of -O when you run `make install`.

http://bugs.python.org/review/23731/diff/14413/Misc/python.man
File Misc/python.man (right):

http://bugs.python.org/review/23731/diff/14413/Misc/python.man#newcode107
Misc/python.man:107: Documentation for installed Python modules and packages can
be
On 2015/04/02 07:19:17, eric.snow wrote:
> a drive-by!  nice :)

ACK

http://bugs.python.org/review/23731/diff/14413/Python/pythonrun.c
File Python/pythonrun.c (left):

http://bugs.python.org/review/23731/diff/14413/Python/pythonrun.c#oldcode376
Python/pythonrun.c:376: Py_OptimizeFlag = 1;
On 2015/04/02 07:19:17, eric.snow wrote:
> I can see how we wouldn't special-case a filename with a "opt-" tag. :)

ACK
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+