[Python-checkins] cpython: Issue #25791: Warn when __package__ != __spec__.parent.
brett.cannon
python-checkins at python.org
Fri Jan 22 18:27:03 EST 2016
https://hg.python.org/cpython/rev/219c44fe8968
changeset: 100051:219c44fe8968
user: Brett Cannon <brett at python.org>
date: Fri Jan 22 15:25:50 2016 -0800
summary:
Issue #25791: Warn when __package__ != __spec__.parent.
In a previous change, __spec__.parent was prioritized over
__package__. That is a backwards-compatibility break, but we do
eventually want __spec__ to be the ground truth for module details. So
this change reverts the change in semantics and instead raises an
ImportWarning when __package__ != __spec__.parent to give people time
to adjust to using spec objects.
files:
Doc/reference/import.rst | 18 +-
Doc/whatsnew/3.6.rst | 6 +-
Lib/importlib/_bootstrap.py | 12 +-
Lib/test/test_importlib/import_/test___package__.py | 33 +-
Lib/test/test_importlib/import_/test_relative_imports.py | 16 +-
Misc/NEWS | 4 +-
Python/import.c | 101 +++++---
Python/importlib.h | 117 +++++----
8 files changed, 185 insertions(+), 122 deletions(-)
diff --git a/Doc/reference/import.rst b/Doc/reference/import.rst
--- a/Doc/reference/import.rst
+++ b/Doc/reference/import.rst
@@ -554,20 +554,30 @@
details.
This attribute is used instead of ``__name__`` to calculate explicit
- relative imports for main modules -- as defined in :pep:`366` --
- when ``__spec__`` is not defined.
+ relative imports for main modules, as defined in :pep:`366`. It is
+ expected to have the same value as ``__spec__.parent``.
+
+ .. versionchanged:: 3.6
+ The value of ``__package__`` is expected to be the same as
+ ``__spec__.parent``.
.. attribute:: __spec__
The ``__spec__`` attribute must be set to the module spec that was
- used when importing the module. This is used primarily for
- introspection and during reloading. Setting ``__spec__``
+ used when importing the module. Setting ``__spec__``
appropriately applies equally to :ref:`modules initialized during
interpreter startup <programs>`. The one exception is ``__main__``,
where ``__spec__`` is :ref:`set to None in some cases <main_spec>`.
+ When ``__package__`` is not defined, ``__spec__.parent`` is used as
+ a fallback.
+
.. versionadded:: 3.4
+ .. versionchanged:: 3.6
+ ``__spec__.parent`` is used as a fallback when ``__package__`` is
+ not defined.
+
.. attribute:: __path__
If the module is a package (either regular or namespace), the module
diff --git a/Doc/whatsnew/3.6.rst b/Doc/whatsnew/3.6.rst
--- a/Doc/whatsnew/3.6.rst
+++ b/Doc/whatsnew/3.6.rst
@@ -274,9 +274,9 @@
:mod:`wave`. This means they will export new symbols when ``import *``
is used. See :issue:`23883`.
-* When performing a relative import, ``__spec__.parent`` is used
- is ``__spec__`` is defined instead of ``__package__``.
- (Contributed by Rose Ames in :issue:`25791`.)
+* When performing a relative import, if ``__package__`` does not compare equal
+ to ``__spec__.parent`` then :exc:`ImportWarning` is raised.
+ (Contributed by Brett Cannon in :issue:`25791`.)
Changes in the C API
diff --git a/Lib/importlib/_bootstrap.py b/Lib/importlib/_bootstrap.py
--- a/Lib/importlib/_bootstrap.py
+++ b/Lib/importlib/_bootstrap.py
@@ -1032,11 +1032,17 @@
to represent that its proper value is unknown.
"""
+ package = globals.get('__package__')
spec = globals.get('__spec__')
- if spec is not None:
+ if package is not None:
+ if spec is not None and package != spec.parent:
+ _warnings.warn("__package__ != __spec__.parent "
+ f"({package!r} != {spec.parent!r})",
+ ImportWarning, stacklevel=3)
+ return package
+ elif spec is not None:
return spec.parent
- package = globals.get('__package__')
- if package is None:
+ else:
_warnings.warn("can't resolve package from __spec__ or __package__, "
"falling back on __name__ and __path__",
ImportWarning, stacklevel=3)
diff --git a/Lib/test/test_importlib/import_/test___package__.py b/Lib/test/test_importlib/import_/test___package__.py
--- a/Lib/test/test_importlib/import_/test___package__.py
+++ b/Lib/test/test_importlib/import_/test___package__.py
@@ -5,6 +5,7 @@
"""
import unittest
+import warnings
from .. import util
@@ -38,8 +39,8 @@
with util.import_state(meta_path=[importer]):
self.__import__('pkg.fake')
module = self.__import__('',
- globals=globals_,
- fromlist=['attr'], level=2)
+ globals=globals_,
+ fromlist=['attr'], level=2)
return module
def test_using___package__(self):
@@ -49,7 +50,10 @@
def test_using___name__(self):
# [__name__]
- module = self.import_module({'__name__': 'pkg.fake', '__path__': []})
+ with warnings.catch_warnings():
+ warnings.simplefilter("ignore")
+ module = self.import_module({'__name__': 'pkg.fake',
+ '__path__': []})
self.assertEqual(module.__name__, 'pkg')
def test_warn_when_using___name__(self):
@@ -58,14 +62,22 @@
def test_None_as___package__(self):
# [None]
- module = self.import_module({
- '__name__': 'pkg.fake', '__path__': [], '__package__': None })
+ with warnings.catch_warnings():
+ warnings.simplefilter("ignore")
+ module = self.import_module({
+ '__name__': 'pkg.fake', '__path__': [], '__package__': None })
self.assertEqual(module.__name__, 'pkg')
- def test_prefers___spec__(self):
- globals = {'__spec__': FakeSpec()}
- with self.assertRaises(SystemError):
- self.__import__('', globals, {}, ['relimport'], 1)
+ def test_spec_fallback(self):
+ # If __package__ isn't defined, fall back on __spec__.parent.
+ module = self.import_module({'__spec__': FakeSpec('pkg.fake')})
+ self.assertEqual(module.__name__, 'pkg')
+
+ def test_warn_when_package_and_spec_disagree(self):
+ # Raise an ImportWarning if __package__ != __spec__.parent.
+ with self.assertWarns(ImportWarning):
+ self.import_module({'__package__': 'pkg.fake',
+ '__spec__': FakeSpec('pkg.fakefake')})
def test_bad__package__(self):
globals = {'__package__': '<not real>'}
@@ -79,7 +91,8 @@
class FakeSpec:
- parent = '<fake>'
+ def __init__(self, parent):
+ self.parent = parent
class Using__package__PEP302(Using__package__):
diff --git a/Lib/test/test_importlib/import_/test_relative_imports.py b/Lib/test/test_importlib/import_/test_relative_imports.py
--- a/Lib/test/test_importlib/import_/test_relative_imports.py
+++ b/Lib/test/test_importlib/import_/test_relative_imports.py
@@ -2,6 +2,8 @@
from .. import util
import sys
import unittest
+import warnings
+
class RelativeImports:
@@ -65,9 +67,11 @@
uncache_names.append(name[:-len('.__init__')])
with util.mock_spec(*create) as importer:
with util.import_state(meta_path=[importer]):
- for global_ in globals_:
- with util.uncache(*uncache_names):
- callback(global_)
+ with warnings.catch_warnings():
+ warnings.simplefilter("ignore")
+ for global_ in globals_:
+ with util.uncache(*uncache_names):
+ callback(global_)
def test_module_from_module(self):
@@ -204,8 +208,10 @@
def test_relative_import_no_globals(self):
# No globals for a relative import is an error.
- with self.assertRaises(KeyError):
- self.__import__('sys', level=1)
+ with warnings.catch_warnings():
+ warnings.simplefilter("ignore")
+ with self.assertRaises(KeyError):
+ self.__import__('sys', level=1)
(Frozen_RelativeImports,
diff --git a/Misc/NEWS b/Misc/NEWS
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -26,8 +26,8 @@
Python 3.5.1 to hide the exact implementation of atomic C types, to avoid
compiler issues.
-- Issue #25791: Trying to resolve a relative import without __spec__ or
- __package__ defined now raises an ImportWarning
+- Issue #25791: If __package__ != __spec__.parent or if neither __package__ or
+ __spec__ are defined then ImportWarning is raised.
- Issue #25731: Fix set and deleting __new__ on a class.
diff --git a/Python/import.c b/Python/import.c
--- a/Python/import.c
+++ b/Python/import.c
@@ -1415,60 +1415,79 @@
goto error;
}
else if (level > 0) {
+ package = _PyDict_GetItemId(globals, &PyId___package__);
spec = _PyDict_GetItemId(globals, &PyId___spec__);
- if (spec != NULL) {
+
+ if (package != NULL && package != Py_None) {
+ Py_INCREF(package);
+ if (!PyUnicode_Check(package)) {
+ PyErr_SetString(PyExc_TypeError, "package must be a string");
+ goto error;
+ }
+ else if (spec != NULL) {
+ int equal;
+ PyObject *parent = PyObject_GetAttrString(spec, "parent");
+ if (parent == NULL) {
+ goto error;
+ }
+
+ equal = PyObject_RichCompareBool(package, parent, Py_EQ);
+ Py_DECREF(parent);
+ if (equal < 0) {
+ goto error;
+ }
+ else if (equal == 0) {
+ if (PyErr_WarnEx(PyExc_ImportWarning,
+ "__package__ != __spec__.parent", 1) < 0) {
+ goto error;
+ }
+ }
+ }
+ }
+ else if (spec != NULL) {
package = PyObject_GetAttrString(spec, "parent");
- }
- if (package != NULL && package != Py_None) {
- if (!PyUnicode_Check(package)) {
+ if (package == NULL) {
+ goto error;
+ }
+ else if (!PyUnicode_Check(package)) {
PyErr_SetString(PyExc_TypeError,
"__spec__.parent must be a string");
goto error;
}
}
else {
- package = _PyDict_GetItemId(globals, &PyId___package__);
- if (package != NULL && package != Py_None) {
- Py_INCREF(package);
- if (!PyUnicode_Check(package)) {
- PyErr_SetString(PyExc_TypeError, "package must be a string");
+ if (PyErr_WarnEx(PyExc_ImportWarning,
+ "can't resolve package from __spec__ or __package__, "
+ "falling back on __name__ and __path__", 1) < 0) {
+ goto error;
+ }
+
+ package = _PyDict_GetItemId(globals, &PyId___name__);
+ if (package == NULL) {
+ PyErr_SetString(PyExc_KeyError, "'__name__' not in globals");
+ goto error;
+ }
+
+ Py_INCREF(package);
+ if (!PyUnicode_Check(package)) {
+ PyErr_SetString(PyExc_TypeError, "__name__ must be a string");
+ goto error;
+ }
+
+ if (_PyDict_GetItemId(globals, &PyId___path__) == NULL) {
+ PyObject *partition = NULL;
+ PyObject *borrowed_dot = _PyUnicode_FromId(&single_dot);
+ if (borrowed_dot == NULL) {
goto error;
}
- }
- else {
- if (PyErr_WarnEx(PyExc_ImportWarning,
- "can't resolve package from __spec__ or __package__, "
- "falling back on __name__ and __path__", 1) < 0) {
+ partition = PyUnicode_RPartition(package, borrowed_dot);
+ Py_DECREF(package);
+ if (partition == NULL) {
goto error;
}
-
- package = _PyDict_GetItemId(globals, &PyId___name__);
- if (package == NULL) {
- PyErr_SetString(PyExc_KeyError, "'__name__' not in globals");
- goto error;
- }
-
+ package = PyTuple_GET_ITEM(partition, 0);
Py_INCREF(package);
- if (!PyUnicode_Check(package)) {
- PyErr_SetString(PyExc_TypeError, "__name__ must be a string");
- goto error;
- }
-
- if (_PyDict_GetItemId(globals, &PyId___path__) == NULL) {
- PyObject *partition = NULL;
- PyObject *borrowed_dot = _PyUnicode_FromId(&single_dot);
- if (borrowed_dot == NULL) {
- goto error;
- }
- partition = PyUnicode_RPartition(package, borrowed_dot);
- Py_DECREF(package);
- if (partition == NULL) {
- goto error;
- }
- package = PyTuple_GET_ITEM(partition, 0);
- Py_INCREF(package);
- Py_DECREF(partition);
- }
+ Py_DECREF(partition);
}
}
diff --git a/Python/importlib.h b/Python/importlib.h
--- a/Python/importlib.h
+++ b/Python/importlib.h
[stripped]
--
Repository URL: https://hg.python.org/cpython
More information about the Python-checkins
mailing list