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

#18864: Implementation for PEP 451 (importlib.machinery.ModuleSpec)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 6 months ago by ericsnowcurrently
Modified:
5 years, 4 months ago
Reviewers:
brett, jimjjewett
CC:
brett.cannon, terry.reedy, Nick Coghlan, larry, Arfrever, Claudiu.Popa, BreamoreBoy, devnull_psf.upfronthosting.co.za, eric.snow, berkerpeksag, pconnell
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Total comments: 19

Patch Set 3 #

Patch Set 4 #

Total comments: 20

Patch Set 5 #

Total comments: 32

Patch Set 6 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/library/importlib.rst View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
Lib/importlib/_bootstrap.py View 1 2 3 4 5 1 chunk +4 lines, -0 lines 0 comments Download
Lib/test/test_importlib/test_spec.py View 1 2 3 4 5 1 chunk +7 lines, -0 lines 0 comments Download
Python/importlib.h View 1 2 3 4 5 2 chunks +2929 lines, -2920 lines 0 comments Download

Messages

Total messages: 8
brett.cannon
Mostly just comments suggesting to make more stuff dealing with the old way of handling ...
5 years, 5 months ago #1
brett.cannon
http://bugs.python.org/review/18864/diff/9695/Doc/reference/import.rst File Doc/reference/import.rst (right): http://bugs.python.org/review/18864/diff/9695/Doc/reference/import.rst#newcode494 Doc/reference/import.rst:494: has been imported. When the module is a package, ...
5 years, 5 months ago #2
eric.snow
http://bugs.python.org/review/18864/diff/9695/Doc/reference/import.rst File Doc/reference/import.rst (right): http://bugs.python.org/review/18864/diff/9695/Doc/reference/import.rst#newcode303 Doc/reference/import.rst:303: Meta path finders in earlier versions of Python implemented ...
5 years, 5 months ago #3
Jim.J.Jewett
http://bugs.python.org/review/18864/diff/9743/Doc/reference/import.rst File Doc/reference/import.rst (right): http://bugs.python.org/review/18864/diff/9743/Doc/reference/import.rst#newcode347 Doc/reference/import.rst:347: elif not hasattr(spec.loader, 'exec_module'): So if spec.loader is None ...
5 years, 5 months ago #4
eric.snow
Thanks for the review, Jim. I'll see about getting a new patch up in the ...
5 years, 5 months ago #5
brett.cannon
I'm doing this review bit-by-bit, so to start here are comments on the docs. http://bugs.python.org/review/18864/diff/9778/Doc/library/importlib.rst ...
5 years, 5 months ago #6
eric.snow
http://bugs.python.org/review/18864/diff/9778/Doc/library/importlib.rst File Doc/library/importlib.rst (right): http://bugs.python.org/review/18864/diff/9778/Doc/library/importlib.rst#newcode899 Doc/library/importlib.rst:899: (``__name__``) On 2013/11/08 21:26:09, brett.cannon wrote: > Why the ...
5 years, 5 months ago #7
eric.snow
5 years, 4 months ago #8
http://bugs.python.org/review/18864/diff/9778/Doc/reference/import.rst
File Doc/reference/import.rst (right):

http://bugs.python.org/review/18864/diff/9778/Doc/reference/import.rst#newcod...
Doc/reference/import.rst:284: :meth:`find_spec()` which takes two arguments, a
name and an import path.
On 2013/11/08 21:26:09, brett.cannon wrote:
> Three now as there is also 'target'.

Done.

http://bugs.python.org/review/18864/diff/9778/Doc/reference/import.rst#newcod...
Doc/reference/import.rst:294: The :meth:`find_spec()` method of meta path
finders is called with two
On 2013/11/08 21:26:09, brett.cannon wrote:
> three arguments

Done.

http://bugs.python.org/review/18864/diff/9778/Doc/reference/import.rst#newcod...
Doc/reference/import.rst:306: ``mpf.find_spec("foo", None)`` on each meta path
finder (``mpf``). After
On 2013/11/08 21:26:09, brett.cannon wrote:
> Technically ('foo', None, None) now (and the other find_spec() calls.

Done.

http://bugs.python.org/review/18864/diff/9778/Doc/reference/import.rst#newcod...
Doc/reference/import.rst:352: module = spec.loader.load_module(spec.name)
On 2013/11/08 21:26:09, brett.cannon wrote:
> # __loader__ and __package__ if missing.

Done.

http://bugs.python.org/review/18864/diff/9778/Doc/reference/import.rst#newcod...
Doc/reference/import.rst:363: module_to_return = sys.modules[spec.name]
On 2013/11/08 21:26:09, brett.cannon wrote:
> return sys.modules[spec.name]

Done.

http://bugs.python.org/review/18864/diff/9778/Doc/reference/import.rst#newcod...
Doc/reference/import.rst:383: sets the import-related module attributes
("init_module_attrs"), as
On 2013/11/08 21:26:09, brett.cannon wrote:
> "(_init_module_attrs() in the pseudocode example above)"

Done.

http://bugs.python.org/review/18864/diff/9778/Doc/reference/import.rst#newcod...
Doc/reference/import.rst:395: loaders.  These were previously performed by the
:meth:`load_module()`
On 2013/11/08 21:26:09, brett.cannon wrote:
> importlib.abc.Loader.load_module()

Done.

http://bugs.python.org/review/18864/diff/9778/Doc/reference/import.rst#newcod...
Doc/reference/import.rst:402: The import machinery calls the
:meth:`~importlib.abc.Loader.exec_module()`
On 2013/11/08 21:26:09, brett.cannon wrote:
> What does the ~ do?
It's equivalent to :meth:`exec_module() <importlib.abc.Loader.exec_module>`.

http://bugs.python.org/review/18864/diff/9778/Doc/reference/import.rst#newcod...
Doc/reference/import.rst:404: returned from exec_module() is ignored.
On 2013/11/08 21:26:09, brett.cannon wrote:
> :meth:...

Done.

http://bugs.python.org/review/18864/diff/9778/Doc/reference/import.rst#newcod...
Doc/reference/import.rst:412: * If loader cannot execute the module, it should
raise an
On 2013/11/08 21:26:09, brett.cannon wrote:
> "If the loader"

Done.

http://bugs.python.org/review/18864/diff/9778/Doc/reference/import.rst#newcod...
Doc/reference/import.rst:744: :meth:`find_spec()` takes one argument, the fully
qualified name of the
On 2013/11/08 21:26:09, brett.cannon wrote:
> Takes two arguments now thanks to 'target'

Done.

http://bugs.python.org/review/18864/diff/9778/Doc/reference/import.rst#newcod...
Doc/reference/import.rst:755: find_spec() replaced find_loader() and
find_module(), but of which
On 2013/11/08 21:26:09, brett.cannon wrote:
> "but" -> "both"

Done.
Sign in to reply to this message.

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