From ab2cb0e4231c8ea1b228e2c168d7f35e7d6bbfcf Mon Sep 17 00:00:00 2001 From: Matthias Bussonnier Date: Tue, 14 Feb 2017 00:20:18 -0800 Subject: [PATCH 1/3] bpo-29546: Set name on ImportError for from ... import And `path` as well when existing. See bpo-29546 This is a step toward providing better error messages in case of from-import. Barry Warsaw Proposed: cannot import name {name} from {module} ({path}) But that's probably going to trigger more discussions that filling in already existing fields. --- Python/ceval.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 66fd361502626f..69c93838419609 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -4995,7 +4995,7 @@ import_from(PyObject *v, PyObject *name) { PyObject *x; _Py_IDENTIFIER(__name__); - PyObject *fullmodname, *pkgname; + PyObject *fullmodname, *pkgname, *pkgpath; x = PyObject_GetAttr(v, name); if (x != NULL || !PyErr_ExceptionMatches(PyExc_AttributeError)) @@ -5021,7 +5021,15 @@ import_from(PyObject *v, PyObject *name) Py_INCREF(x); return x; error: - PyErr_Format(PyExc_ImportError, "cannot import name %R", name); + pkgpath = PyModule_GetFilenameObject(v); + + if (pkgpath == NULL || !PyUnicode_Check(pkgpath)) { + PyErr_Clear(); + PyErr_SetImportError(PyUnicode_FromFormat("cannot import name %R", name), pkgname, NULL); + } else { + PyErr_SetImportError(PyUnicode_FromFormat("cannot import name %R", name), pkgname, pkgpath); + } + return NULL; } From 7456cfc8dfeb6b79675df7ef2bb58fd5ede92bc6 Mon Sep 17 00:00:00 2001 From: Matthias Bussonnier Date: Tue, 14 Feb 2017 12:04:53 -0800 Subject: [PATCH 2/3] Add test for the from ... import behavior. Make sure they name and path are set in various case. --- Lib/test/test_import/__init__.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py index d61782a6cb9ac8..df678f17f18e4e 100644 --- a/Lib/test/test_import/__init__.py +++ b/Lib/test/test_import/__init__.py @@ -80,6 +80,25 @@ def test_from_import_missing_attr_raises_ImportError(self): with self.assertRaises(ImportError): from importlib import something_that_should_not_exist_anywhere + def test_from_import_missing_attr_has_name_and_path(self): + with self.assertRaises(ImportError) as cm: + from os import i_dont_exist + self.assertEqual(cm.exception.name, 'os') + self.assertEqual(cm.exception.path, os.__file__) + + def test_from_import_missing_attr_has_name(self): + with self.assertRaises(ImportError) as cm: + # _warning has no path as it's a built-in module. + from _warning import i_dont_exist + self.assertEqual(cm.exception.name, '_warning') + self.assertIsNone(cm.exception.path) + + def test_from_import_missing_attr_path_is_canonical(self): + with self.assertRaises(ImportError) as cm: + from os.path import i_dont_exist + self.assertIn(cm.exception.name, {'posixpath', 'ntpath'}) + self.assertIsNotNone(cm.exception) + def test_case_sensitivity(self): # Brief digression to test that import is case-sensitive: if we got # this far, we know for sure that "random" exists. From 96b967ab38e147da04ec248a9fa919870fe467a7 Mon Sep 17 00:00:00 2001 From: Matthias Bussonnier Date: Tue, 14 Feb 2017 15:49:13 -0800 Subject: [PATCH 3/3] Add entry into Misc/NEWS --- Misc/NEWS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Misc/NEWS b/Misc/NEWS index e024e01a300b7d..51055ef6f6f5ba 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -12,6 +12,8 @@ Core and Builtins - bpo-29438: Fixed use-after-free problem in key sharing dict. +- bpo-29546: Set the 'path' and 'name' attribute on ImportError for ``from ... import ...``. + - Issue #29319: Prevent RunMainFromImporter overwriting sys.path[0]. - Issue #29337: Fixed possible BytesWarning when compare the code objects.