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-check...@python.org> wrote: > http://hg.python.org/cpython/rev/e39a8f8ceb9f > changeset: 83607:e39a8f8ceb9f > user: Brett Cannon <br...@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-check...@python.org > http://mail.python.org/mailman/listinfo/python-checkins > _______________________________________________ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com