[Python-Dev] [Python-checkins] cpython: #17115, 17116: Have modules initialize the package and loader (original) (raw)

Brett Cannon brett at python.org
Sat May 4 20:49:16 CEST 2013

FYI, I'm aware this broke some buildbots and will have a look today to figure out why.

On Sat, May 4, 2013 at 1:57 PM, brett.cannon <python-checkins at python.org> wrote:

http://hg.python.org/cpython/rev/e39a8f8ceb9f changeset: 83607:e39a8f8ceb9f user: Brett Cannon <brett at python.org> date: Sat May 04 13:56:58 2013 -0400 summary: #17115,17116: Have modules initialize the package and loader attributes to None.

The long-term goal is for people to be able to rely on these attributes existing and checking for None to see if they have been set. Since import itself sets these attributes when a loader does not the only instances when the attributes are None are from someone overloading import() and not using a loader or someone creating a module from scratch. This patch also unifies module initialization. Before you could have different attributes with default values depending on how the module object was created. Now the only way to not get the same default set of attributes is to circumvent initialization by calling ModuleType.new() directly. files: Doc/c-api/module.rst | 11 +- Doc/library/importlib.rst | 2 +- Doc/reference/import.rst | 4 +- Doc/whatsnew/3.4.rst | 5 + Lib/ctypes/test/init.py | 2 +- Lib/doctest.py | 2 +- Lib/importlib/bootstrap.py | 2 +- Lib/inspect.py | 2 +- Lib/test/testdescr.py | 4 +- Lib/test/testimportlib/testapi.py | 14 +- Lib/test/testmodule.py | 28 +- Misc/NEWS | 3 + Objects/moduleobject.c | 39 +- Python/importlib.h | 363 ++++++++------- Python/pythonrun.c | 3 +- 15 files changed, 264 insertions(+), 220 deletions(-)

diff --git a/Doc/c-api/module.rst b/Doc/c-api/module.rst --- a/Doc/c-api/module.rst +++ b/Doc/c-api/module.rst @@ -35,13 +35,20 @@ single: name (module attribute) single: doc (module attribute) single: file (module attribute) + single: package (module attribute) + single: loader (module attribute) Return a new module object with the :attr:_name_ attribute set to name. - Only the module's :attr:_doc_ and :attr:_name_ attributes are filled in; - the caller is responsible for providing a :attr:_file_ attribute. + The module's :attr:_name_, :attr:_doc_, :attr:_package_, and + :attr:_loader_ attributes are filled in (all but :attr:_name_ are set + to None); the caller is responsible for providing a :attr:_file_ + attribute. .. versionadded:: 3.3 + .. versionchanged:: 3.4 + :attr:_package_ and :attr:_loader_ are set to None. + .. c:function:: PyObject* PyModuleNew(const char *name) diff --git a/Doc/library/importlib.rst b/Doc/library/importlib.rst --- a/Doc/library/importlib.rst +++ b/Doc/library/importlib.rst @@ -827,7 +827,7 @@ decorator as it subsumes this functionality. .. versionchanged:: 3.4 - Set _loader_ if set to None as well if the attribute does not + Set _loader_ if set to None, as if the attribute does not exist. diff --git a/Doc/reference/import.rst b/Doc/reference/import.rst --- a/Doc/reference/import.rst +++ b/Doc/reference/import.rst @@ -423,8 +423,8 @@ * If the module has a _file_ attribute, this is used as part of the module's repr. - * If the module has no _file_ but does have a _loader_, then the - loader's repr is used as part of the module's repr. + * If the module has no _file_ but does have a _loader_ that is not + None, then the loader's repr is used as part of the module's repr. * Otherwise, just use the module's _name_ in the repr. diff --git a/Doc/whatsnew/3.4.rst b/Doc/whatsnew/3.4.rst --- a/Doc/whatsnew/3.4.rst +++ b/Doc/whatsnew/3.4.rst @@ -231,3 +231,8 @@ :exc:NotImplementedError blindly. This will only affect code calling :func:super and falling through all the way to the ABCs. For compatibility, catch both :exc:NotImplementedError or the appropriate exception as needed. + +* The module type now initializes the :attr:_package_ and :attr:_loader_ + attributes to None by default. To determine if these attributes were set + in a backwards-compatible fashion, use e.g. + getattr(module, '_loader_', None) is not None. \ No newline at end of file diff --git a/Lib/ctypes/test/init.py b/Lib/ctypes/test/init.py --- a/Lib/ctypes/test/init.py +++ b/Lib/ctypes/test/init.py @@ -37,7 +37,7 @@ def findpackagemodules(package, mask): import fnmatch - if (hasattr(package, "loader") and + if (package.loader is not None and hasattr(package.loader, 'files')): path = package.name.replace(".", os.path.sep) mask = os.path.join(path, mask) diff --git a/Lib/doctest.py b/Lib/doctest.py --- a/Lib/doctest.py +++ b/Lib/doctest.py @@ -215,7 +215,7 @@ if modulerelative: package = normalizemodule(package, 3) filename = modulerelativepath(package, filename) - if hasattr(package, 'loader'): + if getattr(package, 'loader', None) is not None: if hasattr(package.loader, 'getdata'): filecontents = package.loader.getdata(filename) filecontents = filecontents.decode(encoding) diff --git a/Lib/importlib/bootstrap.py b/Lib/importlib/bootstrap.py --- a/Lib/importlib/bootstrap.py +++ b/Lib/importlib/bootstrap.py @@ -1726,7 +1726,7 @@ moduletype = type(sys) for name, module in sys.modules.items(): if isinstance(module, moduletype): - if not hasattr(module, 'loader'): + if getattr(module, 'loader', None) is None: if name in sys.builtinmodulenames: module.loader = BuiltinImporter elif imp.isfrozen(name): diff --git a/Lib/inspect.py b/Lib/inspect.py --- a/Lib/inspect.py +++ b/Lib/inspect.py @@ -476,7 +476,7 @@ if os.path.exists(filename): return filename # only return a non-existent filename if the module has a PEP 302 loader - if hasattr(getmodule(object, filename), 'loader'): + if getattr(getmodule(object, filename), 'loader', None) is not None: return filename # or it is in the linecache if filename in linecache.cache: diff --git a/Lib/test/testdescr.py b/Lib/test/testdescr.py --- a/Lib/test/testdescr.py +++ b/Lib/test/testdescr.py @@ -2250,7 +2250,9 @@ minstance = M("m") minstance.b = 2 minstance.a = 1 - names = [x for x in dir(minstance) if x not in ["name", "doc"]] + defaultattributes = ['name', 'doc', 'package', + 'loader'] + names = [x for x in dir(minstance) if x not in defaultattributes] self.assertEqual(names, ['a', 'b']) class M2(M): diff --git a/Lib/test/testimportlib/testapi.py b/Lib/test/testimportlib/testapi.py --- a/Lib/test/testimportlib/testapi.py +++ b/Lib/test/testimportlib/testapi.py @@ -197,14 +197,12 @@ # Issue #17098: all modules should have loader defined. for name, module in sys.modules.items(): if isinstance(module, types.ModuleType): - if name in sys.builtinmodulenames: - self.assertIn(module.loader, - (importlib.machinery.BuiltinImporter, - importlib.bootstrap.BuiltinImporter)) - elif imp.isfrozen(name): - self.assertIn(module.loader, - (importlib.machinery.FrozenImporter, - importlib.bootstrap.FrozenImporter)) + self.assertTrue(hasattr(module, 'loader'), + '{!r} lacks a loader attribute'.format(name)) + if importlib.machinery.BuiltinImporter.findmodule(name): + self.assertIsNot(module.loader, None) + elif importlib.machinery.FrozenImporter.findmodule(name): + self.assertIsNot(module.loader, None) if name == 'main': diff --git a/Lib/test/testmodule.py b/Lib/test/testmodule.py --- a/Lib/test/testmodule.py +++ b/Lib/test/testmodule.py @@ -33,7 +33,10 @@ foo = ModuleType("foo") self.assertEqual(foo.name, "foo") self.assertEqual(foo.doc, None) - self.assertEqual(foo.dict, {"name": "foo", "doc": None}) + self.assertIs(foo.loader, None) + self.assertIs(foo.package, None) + self.assertEqual(foo.dict, {"name": "foo", "doc": None, + "loader": None, "package": None}) def testasciidocstring(self): # ASCII docstring @@ -41,7 +44,8 @@ self.assertEqual(foo.name, "foo") self.assertEqual(foo.doc, "foodoc") self.assertEqual(foo.dict, - {"name": "foo", "doc": "foodoc"}) + {"name": "foo", "doc": "foodoc", + "loader": None, "package": None}) def testunicodedocstring(self): # Unicode docstring @@ -49,7 +53,8 @@ self.assertEqual(foo.name, "foo") self.assertEqual(foo.doc, "foodoc\u1234") self.assertEqual(foo.dict, - {"name": "foo", "doc": "foodoc\u1234"}) + {"name": "foo", "doc": "foodoc\u1234", + "loader": None, "package": None}) def testreinit(self): # Reinitialization should not replace the dict @@ -61,7 +66,8 @@ self.assertEqual(foo.doc, "foodoc") self.assertEqual(foo.bar, 42) self.assertEqual(foo.dict, - {"name": "foo", "doc": "foodoc", "bar": 42}) + {"name": "foo", "doc": "foodoc", "bar": 42, + "loader": None, "package": None}) self.assertTrue(foo.dict is d) @unittest.expectedFailure @@ -110,13 +116,19 @@ m.file = '/tmp/foo.py' self.assertEqual(repr(m), "<module '?' from '/tmp/foo.py'>") + def testmodulereprwithloaderasNone(self): + m = ModuleType('foo') + assert m.loader is None + self.assertEqual(repr(m), "<module 'foo'>") + def testmodulereprwithbareloaderbutnoname(self): m = ModuleType('foo') del m.name # Yes, a class not an instance. m.loader = BareLoader + loaderrepr = repr(BareLoader) self.assertEqual( - repr(m), "<module '?' (<class 'test.testmodule.BareLoader'>)>") + repr(m), "<module '?' ({})>".format(loaderrepr)) def testmodulereprwithfullloaderbutnoname(self): # m.loader.modulerepr() will fail because the module has no @@ -126,15 +138,17 @@ del m.name # Yes, a class not an instance. m.loader = FullLoader + loaderrepr = repr(FullLoader) self.assertEqual( - repr(m), "<module '?' (<class 'test.testmodule.FullLoader'>)>") + repr(m), "<module '?' ({})>".format(loaderrepr)) def testmodulereprwithbareloader(self): m = ModuleType('foo') # Yes, a class not an instance. m.loader = BareLoader + modulerepr = repr(BareLoader) self.assertEqual( - repr(m), "<module 'foo' (<class 'test.testmodule.BareLoader'>)>") + repr(m), "<module 'foo' ({})>".format(modulerepr)) def testmodulereprwithfullloader(self): m = ModuleType('foo') diff --git a/Misc/NEWS b/Misc/NEWS --- a/Misc/NEWS +++ b/Misc/NEWS @@ -10,6 +10,9 @@ Core and Builtins ----------------- +- Issue #17115,17116: Module initialization now includes setting package and + loader attributes to None. + - Issue #17853: Ensure locals of a class that shadow free variables always win over the closures. diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -26,6 +26,27 @@ }; +static int +moduleinitdict(PyObject *mddict, PyObject *name, PyObject *doc) +{ + if (mddict == NULL) + return -1; + if (doc == NULL) + doc = PyNone; + + if (PyDictSetItemString(mddict, "name", name) != 0) + return -1; + if (PyDictSetItemString(mddict, "doc", doc) != 0) + return -1; + if (PyDictSetItemString(mddict, "package", PyNone) != 0) + return -1; + if (PyDictSetItemString(mddict, "loader", PyNone) != 0) + return -1; + + return 0; +} + + PyObject * PyModuleNewObject(PyObject *name) { @@ -36,13 +57,7 @@ m->mddef = NULL; m->mdstate = NULL; m->mddict = PyDictNew(); - if (m->mddict == NULL) - goto fail; - if (PyDictSetItemString(m->mddict, "name", name) != 0) - goto fail; - if (PyDictSetItemString(m->mddict, "doc", PyNone) != 0) - goto fail; - if (PyDictSetItemString(m->mddict, "package", PyNone) != 0) + if (moduleinitdict(m->mddict, name, NULL) != 0) goto fail; PyObjectGCTrack(m); return (PyObject *)m; @@ -347,9 +362,7 @@ return -1; m->mddict = dict; } - if (PyDictSetItemString(dict, "name", name) < 0)_ _- return -1;_ _- if (PyDictSetItemString(dict, "_doc_", doc) < 0)_ _+ if (moduleinitdict(dict, name, doc) < 0)_ _return -1;_ _return 0;_ _}_ _@@ -380,7 +393,7 @@_ _if (m->mddict != NULL) { loader = PyDictGetItemString(m->mddict, "loader"); } - if (loader != NULL) { + if (loader != NULL && loader != PyNone) { repr = PyObjectCallMethod(loader, "modulerepr", "(O)", (PyObject *)m, NULL); if (repr == NULL) { @@ -404,10 +417,10 @@ filename = PyModuleGetFilenameObject((PyObject *)m); if (filename == NULL) { PyErrClear(); - /* There's no m.file, so if there was an loader, use that in + /* There's no m.file, so if there was a loader, use that in * the repr, otherwise, the only thing you can use is m.name */ - if (loader == NULL) { + if (loader == NULL || loader == PyNone) { repr = PyUnicodeFromFormat("<module %R>", name); } else { diff --git a/Python/importlib.h b/Python/importlib.h --- a/Python/importlib.h +++ b/Python/importlib.h [stripped] diff --git a/Python/pythonrun.c b/Python/pythonrun.c --- a/Python/pythonrun.c +++ b/Python/pythonrun.c @@ -866,7 +866,8 @@ * be set if main gets further initialized later in the startup * process. */ - if (PyDictGetItemString(d, "loader") == NULL) { + PyObject *loader = PyDictGetItemString(d, "loader"); + if (loader == NULL || loader == PyNone) { PyObject *loader = PyObjectGetAttrString(interp->importlib, "BuiltinImporter"); if (loader == NULL) { -- Repository URL: http://hg.python.org/cpython

Python-checkins mailing list Python-checkins at python.org http://mail.python.org/mailman/listinfo/python-checkins

More information about the Python-Dev mailing list