More tests, consistently import bfloat16

Author: abrookins

redisvl/query/aggregate.py

@@ -5,6 +5,10 @@
 from redisvl.query.filter import FilterExpression
 from redisvl.redis.utils import array_to_buffer
 from redisvl.utils.token_escaper import TokenEscaper
+from redisvl.utils.utils import lazy_import
+
+nltk = lazy_import("nltk")
+nltk_stopwords = lazy_import("nltk.corpus.stopwords")
 
 
 class AggregationQuery(AggregateRequest):
@@ -162,19 +166,13 @@ def _set_stopwords(self, stopwords: Optional[Union[str, Set[str]]] = "english"):
         if not stopwords:
             self._stopwords = set()
         elif isinstance(stopwords, str):
-            # Lazy import because nltk is an optional dependency
             try:
-                from redisvl.utils.utils import lazy_import
-
-                nltk = lazy_import("nltk")
-                nltk_stopwords = lazy_import("nltk.corpus.stopwords")
+                nltk.download("stopwords", quiet=True)
+                self._stopwords = set(nltk_stopwords.words(stopwords))
             except ImportError:
                 raise ValueError(
                     f"Loading stopwords for {stopwords} failed: nltk is not installed."
                 )
-            try:
-                nltk.download("stopwords", quiet=True)
-                self._stopwords = set(nltk_stopwords.words(stopwords))
             except Exception as e:
                 raise ValueError(f"Error trying to load {stopwords} from nltk. {e}")
         elif isinstance(stopwords, (Set, List, Tuple)) and all(  # type: ignore

redisvl/query/query.py

@@ -6,7 +6,10 @@
 from redisvl.query.filter import FilterExpression
 from redisvl.redis.utils import array_to_buffer
 from redisvl.utils.token_escaper import TokenEscaper
-from redisvl.utils.utils import denorm_cosine_distance
+from redisvl.utils.utils import denorm_cosine_distance, lazy_import
+
+nltk = lazy_import("nltk")
+nltk_stopwords = lazy_import("nltk.corpus.stopwords")
 
 
 class BaseQuery(RedisQuery):
@@ -893,19 +896,13 @@ def _set_stopwords(self, stopwords: Optional[Union[str, Set[str]]] = "english"):
         if not stopwords:
             self._stopwords = set()
         elif isinstance(stopwords, str):
-            # Lazy import because nltk is an optional dependency
             try:
-                from redisvl.utils.utils import lazy_import
-
-                nltk = lazy_import("nltk")
-                nltk_stopwords = lazy_import("nltk.corpus.stopwords")
+                nltk.download("stopwords", quiet=True)
+                self._stopwords = set(nltk_stopwords.words(stopwords))
             except ImportError:
                 raise ValueError(
                     f"Loading stopwords for {stopwords} failed: nltk is not installed."
                 )
-            try:
-                nltk.download("stopwords", quiet=True)
-                self._stopwords = set(nltk_stopwords.words(stopwords))
             except Exception as e:
                 raise ValueError(f"Error trying to load {stopwords} from nltk. {e}")
         elif isinstance(stopwords, (Set, List, Tuple)) and all(  # type: ignore

redisvl/redis/utils.py

@@ -5,8 +5,6 @@
 
 # Lazy import numpy
 np = lazy_import("numpy")
-# Lazy import ml_dtypes
-bfloat16 = lazy_import("ml_dtypes.bfloat16")
 
 from redisvl.schema.fields import VectorDataType
 
@@ -48,10 +46,9 @@ def array_to_buffer(array: List[float], dtype: str) -> bytes:
 
     # Special handling for bfloat16 which requires explicit import from ml_dtypes
     if dtype.lower() == "bfloat16":
-        # Import ml_dtypes.bfloat16 directly to ensure it's registered with numpy
-        from ml_dtypes import bfloat16 as bf16
+        from ml_dtypes import bfloat16
 
-        return np.array(array, dtype=bf16).tobytes()
+        return np.array(array, dtype=bfloat16).tobytes()
 
     return np.array(array, dtype=dtype.lower()).tobytes()
 
@@ -66,11 +63,11 @@ def buffer_to_array(buffer: bytes, dtype: str) -> List[Any]:
         )
 
     # Special handling for bfloat16 which requires explicit import from ml_dtypes
+    # because otherwise the (lazily imported) numpy is unaware of the type
     if dtype.lower() == "bfloat16":
-        # Import ml_dtypes.bfloat16 directly to ensure it's registered with numpy
-        from ml_dtypes import bfloat16 as bf16
+        from ml_dtypes import bfloat16
 
-        return np.frombuffer(buffer, dtype=bf16).tolist()  # type: ignore[return-value]
+        return np.frombuffer(buffer, dtype=bfloat16).tolist()  # type: ignore[return-value]
 
     return np.frombuffer(buffer, dtype=dtype.lower()).tolist()  # type: ignore[return-value]
 

redisvl/utils/utils.py

@@ -321,15 +321,17 @@ def __getattr__(self, name: str) -> Any:
                     else:
                         # This means we couldn't find the attribute in the module path
                         raise AttributeError(
-                            f"{self._parts[0]} has no attribute '{self._parts[1]}'"
+                            f"module '{self._parts[0]}' has no attribute '{self._parts[1]}'"
                         )
 
             # If we have a module, get the requested attribute
             if hasattr(self._module, name):
                 return getattr(self._module, name)
 
             # If the attribute doesn't exist, raise AttributeError
-            raise AttributeError(f"{self._module_path} has no attribute '{name}'")
+            raise AttributeError(
+                f"module '{self._module_path}' has no attribute '{name}'"
+            )
 
         def __call__(self, *args: Any, **kwargs: Any) -> Any:
             # Import the module if it hasn't been imported yet

tests/unit/test_utils.py

@@ -549,6 +549,26 @@ def test_import_standard_library(self):
         assert "json" in sys.modules
         assert result == '{"key": "value"}'
 
+    def test_cached_module_import(self):
+        """Test that _import_module returns the cached module if it exists"""
+        # Remove the module from sys.modules if it's already imported
+        if "json" in sys.modules:
+            del sys.modules["json"]
+
+        # Lazy import the module
+        json = lazy_import("json")
+
+        # Access an attribute to trigger the import
+        json.dumps
+
+        # The module should now be cached
+        # We need to access the private _import_module method directly
+        # to test the cached path
+        module = json._import_module()
+
+        # Verify that the cached module was returned
+        assert module is json._module
+
     def test_import_already_imported_module(self):
         """Test lazy importing of an already imported module"""
         # Make sure the module is imported
@@ -618,6 +638,17 @@ def test_import_nonexistent_module(self):
 
         assert "Failed to lazily import nonexistent_module_xyz" in str(excinfo.value)
 
+    def test_call_nonexistent_module(self):
+        """Test calling a nonexistent module"""
+        # Lazy import a nonexistent module
+        nonexistent = lazy_import("nonexistent_module_xyz")
+
+        # Calling the nonexistent module should raise ImportError
+        with pytest.raises(ImportError) as excinfo:
+            nonexistent()
+
+        assert "Failed to lazily import nonexistent_module_xyz" in str(excinfo.value)
+
     def test_import_nonexistent_attribute(self):
         """Test lazy importing of a nonexistent attribute"""
         # Lazy import a nonexistent attribute
@@ -631,6 +662,19 @@ def test_import_nonexistent_attribute(self):
             excinfo.value
         )
 
+    def test_getattr_on_nonexistent_attribute_path(self):
+        """Test accessing an attribute on a nonexistent attribute path"""
+        # Lazy import a nonexistent attribute path
+        nonexistent_attr = lazy_import("math.nonexistent_attribute")
+
+        # Accessing an attribute on the nonexistent attribute should raise AttributeError
+        with pytest.raises(AttributeError) as excinfo:
+            nonexistent_attr.some_attribute
+
+        assert "module 'math' has no attribute 'nonexistent_attribute'" in str(
+            excinfo.value
+        )
+
     def test_import_noncallable(self):
         """Test calling a non-callable lazy imported object"""
         # Lazy import a non-callable attribute
@@ -654,3 +698,62 @@ def test_attribute_error(self):
         assert "module 'math' has no attribute 'nonexistent_attribute'" in str(
             excinfo.value
         )
+
+    def test_attribute_error_after_import(self):
+        """Test accessing a nonexistent attribute on a module after it's been imported"""
+        # Create a simple module with a known attribute
+        import types
+
+        test_module = types.ModuleType("test_module")
+        test_module.existing_attr = "exists"
+
+        # Add it to sys.modules so lazy_import can find it
+        sys.modules["test_module"] = test_module
+
+        try:
+            # Lazy import the module
+            lazy_mod = lazy_import("test_module")
+
+            # Access the existing attribute to trigger the import
+            assert lazy_mod.existing_attr == "exists"
+
+            # Now access a nonexistent attribute
+            with pytest.raises(AttributeError) as excinfo:
+                lazy_mod.nonexistent_attribute
+
+            assert (
+                "module 'test_module' has no attribute 'nonexistent_attribute'"
+                in str(excinfo.value)
+            )
+        finally:
+            # Clean up
+            if "test_module" in sys.modules:
+                del sys.modules["test_module"]
+
+    def test_attribute_error_with_direct_module_access(self):
+        """Test accessing a nonexistent attribute by directly setting the _module attribute"""
+        # Get the lazy_import function
+        from redisvl.utils.utils import lazy_import
+
+        # Create a lazy import for a module that doesn't exist yet
+        lazy_mod = lazy_import("test_direct_module")
+
+        # Create a simple object with no __getattr__ method
+        class SimpleObject:
+            pass
+
+        obj = SimpleObject()
+
+        # Directly set the _module attribute to our simple object
+        # This bypasses the normal import mechanism
+        lazy_mod._module = obj
+
+        # Now access a nonexistent attribute
+        # This should go through our LazyModule.__getattr__ and hit line 332
+        with pytest.raises(AttributeError) as excinfo:
+            lazy_mod.nonexistent_attribute
+
+        assert (
+            "module 'test_direct_module' has no attribute 'nonexistent_attribute'"
+            in str(excinfo.value)
+        )