From 965834e8026875ee7e876d42166d3fd7486bcba1 Mon Sep 17 00:00:00 2001 From: Milan Oberkirch Date: Thu, 1 Jun 2017 13:05:21 +1000 Subject: [PATCH 01/12] bpo-30436: Fix exception raised for invalid parent. Changes `find_spec()` to raise `ModuleNotFoundError` instead of `AttributeError` when parent does not exist or is not a package. --- Lib/importlib/util.py | 13 +++++++++---- Lib/test/test_cmd_line_script.py | 2 +- Lib/test/test_importlib/test_util.py | 9 +++++++++ 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/Lib/importlib/util.py b/Lib/importlib/util.py index 6bdf0d445db1f5..52901d5b2080ca 100644 --- a/Lib/importlib/util.py +++ b/Lib/importlib/util.py @@ -84,11 +84,16 @@ def find_spec(name, package=None): if fullname not in sys.modules: parent_name = fullname.rpartition('.')[0] if parent_name: - # Use builtins.__import__() in case someone replaced it. - parent = __import__(parent_name, fromlist=['__path__']) - return _find_spec(fullname, parent.__path__) + try: + parent_path = __import__(parent_name, + fromlist=['__path__']).__path__ + except AttributeError as e: + raise ModuleNotFoundError( + "Error while finding module specification for '{}'.".format( + fullname)) from e else: - return _find_spec(fullname, None) + parent_path = None + return _find_spec(fullname, parent_path) else: module = sys.modules[fullname] if module is None: diff --git a/Lib/test/test_cmd_line_script.py b/Lib/test/test_cmd_line_script.py index 1587daf8f582dd..0d0bcd784d2688 100644 --- a/Lib/test/test_cmd_line_script.py +++ b/Lib/test/test_cmd_line_script.py @@ -427,7 +427,7 @@ def test_dash_m_errors(self): tests = ( ('builtins', br'No code object available'), ('builtins.x', br'Error while finding module specification.*' - br'AttributeError'), + br'ModuleNotFoundError'), ('builtins.x.y', br'Error while finding module specification.*' br'ModuleNotFoundError.*No module named.*not a package'), ('os.path', br'loader.*cannot handle'), diff --git a/Lib/test/test_importlib/test_util.py b/Lib/test/test_importlib/test_util.py index ac18e5c34b87fa..ad2210ee6e2867 100644 --- a/Lib/test/test_importlib/test_util.py +++ b/Lib/test/test_importlib/test_util.py @@ -522,6 +522,15 @@ def test_find_relative_module_missing_package(self): self.assertNotIn(name, sorted(sys.modules)) self.assertNotIn(fullname, sorted(sys.modules)) + def test_ModuleNotFoundError_raised_for_broken_module_name(self): + """ + Test that calling find_spec with broken name raises ModuleNotFoundError. + + See issue 30436 for discussion of this behaviour. + """ + with self.assertRaises(ModuleNotFoundError): + self.util.find_spec('module.name') + (Frozen_FindSpecTests, Source_FindSpecTests From d0e80692051c9b5c322b5806761d1b789083b2e1 Mon Sep 17 00:00:00 2001 From: Milan Oberkirch Date: Thu, 1 Jun 2017 17:12:59 +1000 Subject: [PATCH 02/12] Add name argument to ModuleNotFoundError call. --- Lib/importlib/util.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/importlib/util.py b/Lib/importlib/util.py index 52901d5b2080ca..69b2ecd22f663b 100644 --- a/Lib/importlib/util.py +++ b/Lib/importlib/util.py @@ -89,8 +89,8 @@ def find_spec(name, package=None): fromlist=['__path__']).__path__ except AttributeError as e: raise ModuleNotFoundError( - "Error while finding module specification for '{}'.".format( - fullname)) from e + "Error while finding module specification for '{}'." + .format(fullname), name=fullname) from e else: parent_path = None return _find_spec(fullname, parent_path) From 1697a70f505bfe28ec746c55ee2074001e63295f Mon Sep 17 00:00:00 2001 From: Milan Oberkirch Date: Wed, 7 Jun 2017 16:32:42 +1000 Subject: [PATCH 03/12] Tightening up try-except, docs, cosmetic changes. --- Doc/library/importlib.rst | 4 ++++ Lib/importlib/util.py | 8 ++++---- Lib/test/test_importlib/test_util.py | 7 +------ 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/Doc/library/importlib.rst b/Doc/library/importlib.rst index 1df613076da8cf..4f841100cf1cbb 100644 --- a/Doc/library/importlib.rst +++ b/Doc/library/importlib.rst @@ -1215,6 +1215,10 @@ an :term:`importer`. .. versionadded:: 3.4 + .. versionchanged:: 3.7 + Raises :exc:`ModuleNotFoundError` instead of :exc:`AttributeError` if + **package** can be imported but is missing the :attr:`__path__` attribute. + .. function:: module_from_spec(spec) Create a new module based on **spec** and diff --git a/Lib/importlib/util.py b/Lib/importlib/util.py index 69b2ecd22f663b..681f44b84c94d0 100644 --- a/Lib/importlib/util.py +++ b/Lib/importlib/util.py @@ -84,13 +84,13 @@ def find_spec(name, package=None): if fullname not in sys.modules: parent_name = fullname.rpartition('.')[0] if parent_name: + parent = __import__(parent_name, fromlist=['__path__']) try: - parent_path = __import__(parent_name, - fromlist=['__path__']).__path__ + parent_path = parent.__path__ except AttributeError as e: raise ModuleNotFoundError( - "Error while finding module specification for '{}'." - .format(fullname), name=fullname) from e + f"Error while finding module specification for " + f"'{fullname}'.", name=fullname) from e else: parent_path = None return _find_spec(fullname, parent_path) diff --git a/Lib/test/test_importlib/test_util.py b/Lib/test/test_importlib/test_util.py index ad2210ee6e2867..90d6ec6a3cfd1c 100644 --- a/Lib/test/test_importlib/test_util.py +++ b/Lib/test/test_importlib/test_util.py @@ -522,12 +522,7 @@ def test_find_relative_module_missing_package(self): self.assertNotIn(name, sorted(sys.modules)) self.assertNotIn(fullname, sorted(sys.modules)) - def test_ModuleNotFoundError_raised_for_broken_module_name(self): - """ - Test that calling find_spec with broken name raises ModuleNotFoundError. - - See issue 30436 for discussion of this behaviour. - """ + def test_find_submodule_in_module(self): with self.assertRaises(ModuleNotFoundError): self.util.find_spec('module.name') From c6e78512bac9c94e374782e520576d453e46564b Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Wed, 7 Jun 2017 14:43:09 -0700 Subject: [PATCH 04/12] Drop an unnecessary 'f' prefix --- Lib/importlib/util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/importlib/util.py b/Lib/importlib/util.py index 681f44b84c94d0..e0b81392df710c 100644 --- a/Lib/importlib/util.py +++ b/Lib/importlib/util.py @@ -89,7 +89,7 @@ def find_spec(name, package=None): parent_path = parent.__path__ except AttributeError as e: raise ModuleNotFoundError( - f"Error while finding module specification for " + "Error while finding module specification for " f"'{fullname}'.", name=fullname) from e else: parent_path = None From c8438b290cd171b6dd2f958cf416093363d55d36 Mon Sep 17 00:00:00 2001 From: Milan Oberkirch Date: Wed, 14 Jun 2017 20:40:04 +0700 Subject: [PATCH 05/12] Add NEWS entry for bpo-30436. --- Misc/NEWS | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Misc/NEWS b/Misc/NEWS index cda5ce0311ed5b..e9d34895e59995 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -345,6 +345,10 @@ Extension Modules Library ------- +- bpo-30436: `importlib.find_spec()` raises ModuleNotFoundError instead of + AttributeError if the package can be imported but is missing the __path__ + attribute. + - bpo-16500: Allow registering at-fork handlers. - bpo-30470: Deprecate invalid ctypes call protection on Windows. Patch by From 6fb807eedae10010ff7aaf951dbf7434e0473804 Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Wed, 14 Jun 2017 14:04:50 -0700 Subject: [PATCH 06/12] Fix merge conflict for NEWS --- Misc/NEWS | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Misc/NEWS b/Misc/NEWS index e9d34895e59995..28ee5c931181ea 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -345,10 +345,6 @@ Extension Modules Library ------- -- bpo-30436: `importlib.find_spec()` raises ModuleNotFoundError instead of - AttributeError if the package can be imported but is missing the __path__ - attribute. - - bpo-16500: Allow registering at-fork handlers. - bpo-30470: Deprecate invalid ctypes call protection on Windows. Patch by @@ -363,6 +359,10 @@ Library - bpo-30149: inspect.signature() now supports callables with variable-argument parameters wrapped with partialmethod. Patch by Dong-hee Na. + +- bpo-30436: importlib.find_spec() raises ModuleNotFoundError instead of + AttributeError if the specified parent module is not a package + (i.e. lacks a __path__ attribute). - bpo-30301: Fix AttributeError when using SimpleQueue.empty() under *spawn* and *forkserver* start methods. From 0723016a1fccb2c9f7cc9d2af82766971c6f2dbd Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Wed, 14 Jun 2017 14:07:16 -0700 Subject: [PATCH 07/12] Tweak documentation wording --- Doc/library/importlib.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Doc/library/importlib.rst b/Doc/library/importlib.rst index 4f841100cf1cbb..45a02e568e1ba5 100644 --- a/Doc/library/importlib.rst +++ b/Doc/library/importlib.rst @@ -1217,7 +1217,8 @@ an :term:`importer`. .. versionchanged:: 3.7 Raises :exc:`ModuleNotFoundError` instead of :exc:`AttributeError` if - **package** can be imported but is missing the :attr:`__path__` attribute. + **package** is in fact not a package (i.e. lacks a :attr:`__path__` + attribute). .. function:: module_from_spec(spec) From 9f2844c6316bd4fbedc00554ec0e511825557c17 Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Wed, 14 Jun 2017 14:08:51 -0700 Subject: [PATCH 08/12] Tweak exception message --- Lib/importlib/util.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/importlib/util.py b/Lib/importlib/util.py index e0b81392df710c..33b5015e42f832 100644 --- a/Lib/importlib/util.py +++ b/Lib/importlib/util.py @@ -89,8 +89,8 @@ def find_spec(name, package=None): parent_path = parent.__path__ except AttributeError as e: raise ModuleNotFoundError( - "Error while finding module specification for " - f"'{fullname}'.", name=fullname) from e + "__path__ attribute missing on parent of " + f"{fullname!r}.", name=fullname) from e else: parent_path = None return _find_spec(fullname, parent_path) From 9d9c57df2f6d2f9ef5ede39f8fc015297293d1cf Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Wed, 14 Jun 2017 14:10:38 -0700 Subject: [PATCH 09/12] Be very explicit in exception message --- Lib/importlib/util.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/importlib/util.py b/Lib/importlib/util.py index 33b5015e42f832..ef8e7552f7bdab 100644 --- a/Lib/importlib/util.py +++ b/Lib/importlib/util.py @@ -89,8 +89,8 @@ def find_spec(name, package=None): parent_path = parent.__path__ except AttributeError as e: raise ModuleNotFoundError( - "__path__ attribute missing on parent of " - f"{fullname!r}.", name=fullname) from e + f"__path__ attribute not found on {parent_name!r}" + f"while trying to import {fullname!r}.", name=fullname) from e else: parent_path = None return _find_spec(fullname, parent_path) From 6819f9d67197994eaabc41dedb9fa4d210825851 Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Wed, 14 Jun 2017 14:11:34 -0700 Subject: [PATCH 10/12] Clarify exception message --- Lib/importlib/util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/importlib/util.py b/Lib/importlib/util.py index ef8e7552f7bdab..96305da471331e 100644 --- a/Lib/importlib/util.py +++ b/Lib/importlib/util.py @@ -90,7 +90,7 @@ def find_spec(name, package=None): except AttributeError as e: raise ModuleNotFoundError( f"__path__ attribute not found on {parent_name!r}" - f"while trying to import {fullname!r}.", name=fullname) from e + f"while trying to find {fullname!r}.", name=fullname) from e else: parent_path = None return _find_spec(fullname, parent_path) From 24fb8e2d5f6a92b681fcd2e3fabf5fbaf131d8dd Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Wed, 14 Jun 2017 14:12:16 -0700 Subject: [PATCH 11/12] Drop a period from the exception message --- Lib/importlib/util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/importlib/util.py b/Lib/importlib/util.py index 96305da471331e..41c74d4cc6c2a6 100644 --- a/Lib/importlib/util.py +++ b/Lib/importlib/util.py @@ -90,7 +90,7 @@ def find_spec(name, package=None): except AttributeError as e: raise ModuleNotFoundError( f"__path__ attribute not found on {parent_name!r}" - f"while trying to find {fullname!r}.", name=fullname) from e + f"while trying to find {fullname!r}", name=fullname) from e else: parent_path = None return _find_spec(fullname, parent_path) From f6a75fbbb0d3c52dcc167e6e3fc37a02e11f1d5a Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Wed, 14 Jun 2017 14:13:18 -0700 Subject: [PATCH 12/12] Explain what the test is checking for --- Lib/test/test_importlib/test_util.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Lib/test/test_importlib/test_util.py b/Lib/test/test_importlib/test_util.py index 90d6ec6a3cfd1c..56a0b0e7a518cd 100644 --- a/Lib/test/test_importlib/test_util.py +++ b/Lib/test/test_importlib/test_util.py @@ -523,6 +523,8 @@ def test_find_relative_module_missing_package(self): self.assertNotIn(fullname, sorted(sys.modules)) def test_find_submodule_in_module(self): + # ModuleNotFoundError raised when a module is specified as + # a parent instead of a package. with self.assertRaises(ModuleNotFoundError): self.util.find_spec('module.name')