[Python-Dev] [Python-checkins] cpython: #17115, 17116: Have modules initialize the __package__ and __loader__
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/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 at python.org
> http://mail.python.org/mailman/listinfo/python-checkins
>
More information about the Python-Dev
mailing list