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@python.org> wrote:

http://hg.python.org/cpython/rev/e39a8f8ceb9f changeset: 83607:e39a8f8ceb9f user: Brett Cannon <brett@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/test_descr.py | 4 +- Lib/test/test_importlib/test_api.py | 14 +- Lib/test/test_module.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* PyModule_New(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 find_package_modules(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 module_relative: package = _normalize_module(package, 3) filename = _module_relative_path(package, filename) - if hasattr(package, '__loader__'): + if getattr(package, '__loader__', None) is not None: if hasattr(package.__loader__, 'get_data'): file_contents = package.__loader__.get_data(filename) file_contents = file_contents.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 @@ module_type = type(sys) for name, module in sys.modules.items(): if isinstance(module, module_type): - if not hasattr(module, '__loader__'): + if getattr(module, '__loader__', None) is None: if name in sys.builtin_module_names: module.__loader__ = BuiltinImporter elif _imp.is_frozen(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/test_descr.py b/Lib/test/test_descr.py --- a/Lib/test/test_descr.py +++ b/Lib/test/test_descr.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__"]] + default_attributes = ['__name__', '__doc__', '__package__', + '__loader__'] + names = [x for x in dir(minstance) if x not in default_attributes] self.assertEqual(names, ['a', 'b'])

class M2(M): diff --git a/Lib/test/test_importlib/test_api.py b/Lib/test/test_importlib/test_api.py --- a/Lib/test/test_importlib/test_api.py +++ b/Lib/test/test_importlib/test_api.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.builtin_module_names: - self.assertIn(module.__loader__, - (importlib.machinery.BuiltinImporter, - importlib._bootstrap.BuiltinImporter)) - elif imp.is_frozen(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.find_module(name): + self.assertIsNot(module.__loader__, None) + elif importlib.machinery.FrozenImporter.find_module(name): + self.assertIsNot(module.__loader__, None)

if __name__ == '__main__': diff --git a/Lib/test/test_module.py b/Lib/test/test_module.py --- a/Lib/test/test_module.py +++ b/Lib/test/test_module.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 test_ascii_docstring(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 test_unicode_docstring(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 test_reinit(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 test_module_repr_with_loader_as_None(self): + m = ModuleType('foo') + assert m.__loader__ is None + self.assertEqual(repr(m), "<module 'foo'>") + def test_module_repr_with_bare_loader_but_no_name(self): m = ModuleType('foo') del m.__name__ # Yes, a class not an instance. m.__loader__ = BareLoader + loader_repr = repr(BareLoader) self.assertEqual( - repr(m), "<module '?' (<class 'test.test_module.BareLoader'>)>") + repr(m), "<module '?' ({})>".format(loader_repr))

def test_module_repr_with_full_loader_but_no_name(self): # m.__loader__.module_repr() will fail because the module has no @@ -126,15 +138,17 @@ del m.__name__ # Yes, a class not an instance. m.__loader__ = FullLoader + loader_repr = repr(FullLoader) self.assertEqual( - repr(m), "<module '?' (<class 'test.test_module.FullLoader'>)>") + repr(m), "<module '?' ({})>".format(loader_repr))

def test_module_repr_with_bare_loader(self): m = ModuleType('foo') # Yes, a class not an instance. m.__loader__ = BareLoader + module_repr = repr(BareLoader) self.assertEqual( - repr(m), "<module 'foo' (<class 'test.test_module.BareLoader'>)>") + repr(m), "<module 'foo' ({})>".format(module_repr))

def test_module_repr_with_full_loader(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 +module_init_dict(PyObject *md_dict, PyObject *name, PyObject *doc) +{ + if (md_dict == NULL) + return -1; + if (doc == NULL) + doc = Py_None; + + if (PyDict_SetItemString(md_dict, "__name__", name) != 0) + return -1; + if (PyDict_SetItemString(md_dict, "__doc__", doc) != 0) + return -1; + if (PyDict_SetItemString(md_dict, "__package__", Py_None) != 0) + return -1; + if (PyDict_SetItemString(md_dict, "__loader__", Py_None) != 0) + return -1; + + return 0; +} + + PyObject * PyModule_NewObject(PyObject *name) { @@ -36,13 +57,7 @@ m->md_def = NULL; m->md_state = NULL; m->md_dict = PyDict_New(); - if (m->md_dict == NULL) - goto fail; - if (PyDict_SetItemString(m->md_dict, "__name__", name) != 0) - goto fail; - if (PyDict_SetItemString(m->md_dict, "__doc__", Py_None) != 0) - goto fail; - if (PyDict_SetItemString(m->md_dict, "__package__", Py_None) != 0) + if (module_init_dict(m->md_dict, name, NULL) != 0) goto fail; PyObject_GC_Track(m); return (PyObject *)m; @@ -347,9 +362,7 @@ return -1; m->md_dict = dict; } - if (PyDict_SetItemString(dict, "__name__", name) < 0) - return -1; - if (PyDict_SetItemString(dict, "__doc__", doc) < 0) + if (module_init_dict(dict, name, doc) < 0) return -1; return 0; } @@ -380,7 +393,7 @@ if (m->md_dict != NULL) { loader = PyDict_GetItemString(m->md_dict, "__loader__"); } - if (loader != NULL) { + if (loader != NULL && loader != Py_None) { repr = PyObject_CallMethod(loader, "module_repr", "(O)", (PyObject *)m, NULL); if (repr == NULL) { @@ -404,10 +417,10 @@ filename = PyModule_GetFilenameObject((PyObject *)m); if (filename == NULL) { PyErr_Clear(); - /* 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 == Py_None) { repr = PyUnicode_FromFormat("<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 (PyDict_GetItemString(d, "__loader__") == NULL) { + PyObject *loader = PyDict_GetItemString(d, "__loader__"); + if (loader == NULL || loader == Py_None) { PyObject *loader = PyObject_GetAttrString(interp->importlib, "BuiltinImporter"); if (loader == NULL) {

-- Repository URL: http://hg.python.org/cpython

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