Merge pull request #5 from DavidCEllis/improve_coverage

Prevent relative imports without an assigned name. 

Force asname values to be valid python identifiers.

Remove unreachable exceptions where python will error in the import.

Fix an infinite loop on invalid input data.

Add eager_process parameter to force early processing of imports.
Excluded from repr.

Add globs to LazyImporter repr.

Fix get_importer_state bug with lazy processing of imports.

Add tests for these.

pyproject.toml

@@ -34,3 +34,6 @@ where = ["src"]
 
 [tool.setuptools.dynamic]
 version = {attr = "ducktools.lazyimporter.__version__"}
+
+[tool.black]
+skip-string-normalization = true

src/ducktools/lazyimporter.py

@@ -102,6 +102,14 @@ def __init__(self, module_name, asname=None):
         self.module_name = module_name
         self.asname = asname
 
+        if self.import_level > 0 and asname is None:
+            raise ValueError(
+                "Relative imports are not allowed without an assigned name."
+            )
+
+        if self.asname is not None and not asname.isidentifier():
+            raise ValueError(f"{self.asname!r} is not a valid Python identifier.")
+
     def __repr__(self):
         return (
             f"{self.__class__.__name__}("
@@ -126,15 +134,11 @@ def do_import(self, globs=None):
             submod_used = [self.module_basename]
             for submod in self.submodule_names:
                 submod_used.append(submod)
-                try:
-                    mod = getattr(mod, submod)
-                except AttributeError:
-                    invalid_module = ".".join(submod_used)
-                    raise ModuleNotFoundError(f"No module named {invalid_module!r}")
+                mod = getattr(mod, submod)
 
         if self.asname:
             return {self.asname: mod}
-        else:
+        else:  # pragma: no cover
             return {self.module_basename: mod}
 
 
@@ -159,6 +163,9 @@ def __init__(self, module_name, attrib_name, asname=None):
         self.attrib_name = attrib_name
         self.asname = asname if asname is not None else attrib_name
 
+        if not self.asname.isidentifier():
+            raise ValueError(f"{self.asname!r} is not a valid Python identifier.")
+
     def __repr__(self):
         return (
             f"{self.__class__.__name__}("
@@ -208,6 +215,10 @@ def __init__(self, module_name, attrib_names):
         self.module_name = module_name
         self.attrib_names = attrib_names
 
+        for name in self.asnames:
+            if not name.isidentifier():
+                raise ValueError(f"{name!r} is not a valid Python identifier.")
+
     def __repr__(self):
         return (
             f"{self.__class__.__name__}("
@@ -226,6 +237,7 @@ def __eq__(self, other):
     @property
     def asnames(self):
         """
+        Get a list of all the names that will be assigned by this importer
 
         :return: list of 'asname' names to give as 'dir' for LazyImporter bindings
         :rtype: list[str]
@@ -276,14 +288,20 @@ def __init__(self, module_name, except_module, asname):
         Inside a LazyImporter
 
         :param module_name: Name of the 'try' module
+        :type module_name: str
         :param except_module: Name of the module to import in the case
                               that the 'try' module fails
+        :type except_module: str
         :param asname: Name to use for either on successful import
+        :type asname: str
         """
         self.module_name = module_name
         self.except_module = except_module
         self.asname = asname
 
+        if not self.asname.isidentifier():
+            raise ValueError(f"{self.asname!r} is not a valid Python identifier.")
+
     def __repr__(self):
         return (
             f"{self.__class__.__name__}("
@@ -362,11 +380,7 @@ def do_import(self, globs=None):
 
         for submod in submodule_names:
             submod_used.append(submod)
-            try:
-                mod = getattr(mod, submod)
-            except AttributeError:
-                invalid_module = ".".join(submod_used)
-                raise ModuleNotFoundError(f"No module named {invalid_module!r}")
+            mod = getattr(mod, submod)
 
         return {self.asname: mod}
 
@@ -459,7 +473,7 @@ def group_importers(inst):
         importers = {}
 
         for imp in inst._imports:  # noqa
-            if imp.import_level > 0 and inst._globals is None:  # noqa
+            if getattr(imp, "import_level", 0) > 0 and inst._globals is None:  # noqa
                 raise ValueError(
                     "Attempted to setup relative import without providing globals()."
                 )
@@ -501,7 +515,7 @@ def group_importers(inst):
                         )
             else:
                 raise TypeError(
-                    f"{imp} is not an instance of "
+                    f"{imp!r} is not an instance of "
                     f"ModuleImport, FromImport, MultiFromImport or TryExceptImport"
                 )
         return importers
@@ -513,7 +527,7 @@ class LazyImporter:
 
     _importers = _ImporterGrouper()
 
-    def __init__(self, imports, *, globs=None):
+    def __init__(self, imports, *, globs=None, eager_process=False):
         """
         Create a LazyImporter to import modules and objects when they are accessed
         on this importer object.
@@ -524,11 +538,16 @@ def __init__(self, imports, *, globs=None):
         :type imports: list[ModuleImport | FromImport | MultiFromImport | TryExceptImport]
         :param globs: globals object for relative imports
         :type globs: dict[str, typing.Any]
+        :param eager_process: filter and check the imports eagerly
+        :type eager_process: bool
         """
         # Keep original imports for __repr__
         self._imports = imports
         self._globals = globs
 
+        if eager_process:
+            _ = self._importers
+
     def __getattr__(self, name):
         # This performs the imports associated with the name of the attribute
         # and sets the result to that name.
@@ -553,7 +572,11 @@ def __dir__(self):
         return sorted(self._importers.keys())
 
     def __repr__(self):
-        return f"{self.__class__.__name__}(imports={self._imports!r})"
+        return (
+            f"{self.__class__.__name__}"
+            f"(imports={self._imports!r}, "
+            f"globs={self._globals!r})"
+        )
 
 
 def get_importer_state(importer):
@@ -565,10 +588,16 @@ def get_importer_state(importer):
     :return: Dict of imported_modules and lazy_modules
     :rtype: dict[str, dict[str, typing.Any] | list[str]]
     """
+    # Get the dir *before* looking at __dict__
+    # Calling 'dir' in the block would cause the __dict__ size to change
+    # And fail iteration
+    importer_dir = dir(importer)
+
     imported_attributes = {
-        k: v for k, v in importer.__dict__.items() if k in dir(importer)
+        k: v for k, v in importer.__dict__.items() if k in importer_dir
     }
-    lazy_attributes = [k for k in dir(importer) if k not in imported_attributes]
+
+    lazy_attributes = [k for k in importer_dir if k not in imported_attributes]
 
     return {
         "imported_attributes": imported_attributes,

tests/example_modules/ex_othermod/__init__.py

@@ -1,11 +1,14 @@
 from ducktools.lazyimporter import (
     LazyImporter,
     FromImport,
+    get_module_funcs,
 )
 
 name = "ex_othermod"
 
 laz = LazyImporter(
-    [FromImport("..ex_mod.ex_submod", "name")],
+    [FromImport("..ex_mod.ex_submod", "name", "submod_name")],
     globs=globals(),
 )
+
+__getattr__, __dir__ = get_module_funcs(laz, module_name=__name__)

tests/test_basic_imports.py

@@ -31,6 +31,13 @@ def test_module_import(self):
 
         assert example_1 is laz.example_1
 
+    def test_module_import_asname(self):
+        laz = LazyImporter([ModuleImport("example_1", asname="ex1")])
+
+        import example_1  # noqa  # pyright: ignore
+
+        assert example_1 is laz.ex1
+
     def test_from_import(self):
         """
         Test a basic from import from a module
@@ -68,6 +75,11 @@ def test_imports_submod(self):
         assert laz_sub.ex_mod.name == "ex_mod"
         assert laz_sub.ex_mod.ex_submod.name == "ex_submod"
 
+    def test_imports_submod_asname(self):
+        laz_sub = LazyImporter([ModuleImport("ex_mod.ex_submod", asname="ex_submod")])
+
+        assert laz_sub.ex_submod.name == "ex_submod"
+
     def test_submod_from(self):
         """
         Test a from import from a submodule
@@ -115,8 +127,22 @@ def test_try_except_import(self):
 
         assert laz2.ex_mod.name == "ex_mod"
 
+    def test_try_except_submod_import(self):
+        """
+        Test a try/except import with submodules
+        """
+        laz = LazyImporter(
+            [
+                TryExceptImport(
+                    "module_does_not_exist", "ex_mod.ex_submod", "ex_submod"
+                ),
+            ]
+        )
 
-class TestImportsWithinExamples:
+        assert laz.ex_submod.name == "ex_submod"
+
+
+class TestRelativeImports:
     def test_relative_import(self):
         import example_modules.lazy_submod_ex as lse
 
@@ -130,4 +156,4 @@ def test_relative_import(self):
     def test_submod_relative_import(self):
         from example_modules.ex_othermod import laz
 
-        assert laz.name == "ex_submod"
+        assert laz.submod_name == "ex_submod"

tests/test_funcs.py

@@ -0,0 +1,61 @@
+"""
+Test the external functions
+"""
+
+from ducktools.lazyimporter import (
+    ModuleImport,
+    FromImport,
+    MultiFromImport,
+    TryExceptImport,
+    _SubmoduleImports,
+    MultiFromImport,
+    get_importer_state,
+    get_module_funcs,
+    LazyImporter,
+)
+
+
+class TestImporterState:
+    def test_module_importer_state(self):
+        laz = LazyImporter([ModuleImport("collections")])
+
+        state = get_importer_state(laz)
+
+        assert state["lazy_attributes"] == ["collections"]
+        assert state["imported_attributes"] == {}
+
+        collections_mod = laz.collections
+
+        state = get_importer_state(laz)
+
+        assert state["lazy_attributes"] == []
+        assert state["imported_attributes"] == {"collections": collections_mod}
+
+
+class TestModuleFuncs:
+    def test_getattr_func(self):
+        import collections
+
+        laz = LazyImporter([ModuleImport("collections")])
+
+        getattr_func, _ = get_module_funcs(laz)
+
+        assert getattr_func("collections") is collections
+
+    def test_dir_func(self):
+        laz = LazyImporter([ModuleImport("collections")])
+
+        _, dir_func = get_module_funcs(laz)
+
+        assert dir_func() == ["collections"]
+
+    def test_getattr_module_func(self):
+        import example_modules.ex_othermod as ex_othermod  # noqa  # pyright: ignore
+
+        assert ex_othermod.submod_name == "ex_submod"
+
+    def test_dir_module_func(self):
+        import example_modules.ex_othermod as ex_othermod  # noqa  # pyright: ignore
+
+        assert "name" in dir(ex_othermod)
+        assert "submod_name" in dir(ex_othermod)