diff --git a/redisvl/utils/utils.py b/redisvl/utils/utils.py index da1067d8..164fb2ac 100644 --- a/redisvl/utils/utils.py +++ b/redisvl/utils/utils.py @@ -1,4 +1,7 @@ +import inspect import json +import warnings +from contextlib import ContextDecorator, contextmanager from enum import Enum from functools import wraps from time import time @@ -67,24 +70,65 @@ def deprecated_argument(argument: str, replacement: Optional[str] = None) -> Cal When the wrapped function is called, the decorator will warn if the deprecated argument is passed as an argument or keyword argument. - """ + NOTE: The @deprecated_argument decorator should not fall "outside" + of the @classmethod decorator, but instead should come between + it and the method definition. For example: + + class MyClass: + @classmethod + @deprecated_argument("old_arg", "new_arg") + @other_decorator + def test_method(cls, old_arg=None, new_arg=None): + pass + """ message = f"Argument {argument} is deprecated and will be removed in the next major release." if replacement: message += f" Use {replacement} instead." - def wrapper(func): - @wraps(func) - def inner(*args, **kwargs): - argument_names = func.__code__.co_varnames + def decorator(func): + # Check if the function is a classmethod or staticmethod + if isinstance(func, (classmethod, staticmethod)): + underlying = func.__func__ + + @wraps(underlying) + def inner_wrapped(*args, **kwargs): + if argument in kwargs: + warn(message, DeprecationWarning, stacklevel=2) + else: + sig = inspect.signature(underlying) + bound_args = sig.bind(*args, **kwargs) + if argument in bound_args.arguments: + warn(message, DeprecationWarning, stacklevel=2) + return underlying(*args, **kwargs) + + if isinstance(func, classmethod): + return classmethod(inner_wrapped) + else: + return staticmethod(inner_wrapped) + else: - if argument in argument_names: - warn(message, DeprecationWarning, stacklevel=2) - elif argument in kwargs: - warn(message, DeprecationWarning, stacklevel=2) + @wraps(func) + def inner_normal(*args, **kwargs): + if argument in kwargs: + warn(message, DeprecationWarning, stacklevel=2) + else: + sig = inspect.signature(func) + bound_args = sig.bind(*args, **kwargs) + if argument in bound_args.arguments: + warn(message, DeprecationWarning, stacklevel=2) + return func(*args, **kwargs) - return func(*args, **kwargs) + return inner_normal - return inner + return decorator - return wrapper + +@contextmanager +def assert_no_warnings(): + """ + Assert that a function does not emit any warnings when called. + """ + with warnings.catch_warnings(): + warnings.simplefilter("error") + yield diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index cd8a3c9f..4be3c556 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -1,3 +1,6 @@ +import warnings +from functools import wraps + import numpy as np import pytest @@ -7,7 +10,7 @@ convert_bytes, make_dict, ) -from redisvl.utils.utils import deprecated_argument +from redisvl.utils.utils import assert_no_warnings, deprecated_argument def test_even_number_of_elements(): @@ -148,92 +151,264 @@ def test_conversion_with_invalid_floats(): assert len(result) > 0 # Simple check to ensure it returns anything +def decorator(func): + @wraps(func) + def wrapper(*args, **kwargs): + print("boop") + return func(*args, **kwargs) + + return wrapper + + class TestDeprecatedArgument: def test_deprecation_warning_text_with_replacement(self): - @deprecated_argument("dtype", "vectorizer") - def test_func(dtype=None, vectorizer=None): + @deprecated_argument("old_arg", "new_arg") + def test_func(old_arg=None, new_arg=None, neutral_arg=None): pass + # Test that passing the deprecated argument as a keyword triggers the warning. + with pytest.warns(DeprecationWarning) as record: + test_func(old_arg="float32") + + assert len(record) == 1 + assert str(record[0].message) == ( + "Argument old_arg is deprecated and will be removed in the next major release. Use new_arg instead." + ) + + # Test that passing the deprecated argument as a positional argument also triggers the warning. with pytest.warns(DeprecationWarning) as record: - test_func(dtype="float32") + test_func("float32", neutral_arg="test_vector") assert len(record) == 1 assert str(record[0].message) == ( - "Argument dtype is deprecated and will be removed" - " in the next major release. Use vectorizer instead." + "Argument old_arg is deprecated and will be removed in the next major release. Use new_arg instead." ) + with assert_no_warnings(): + test_func(new_arg="float32") + test_func(new_arg="float32", neutral_arg="test_vector") + def test_deprecation_warning_text_without_replacement(self): - @deprecated_argument("dtype") - def test_func(dtype=None): + @deprecated_argument("old_arg") + def test_func(old_arg=None, neutral_arg=None): pass + # As a kwarg with pytest.warns(DeprecationWarning) as record: - test_func(dtype="float32") + test_func(old_arg="float32") assert len(record) == 1 assert str(record[0].message) == ( - "Argument dtype is deprecated and will be removed" + "Argument old_arg is deprecated and will be removed" " in the next major release." ) - def test_function_argument(self): - @deprecated_argument("dtype", "vectorizer") - def test_func(dtype=None, vectorizer=None): + # As a positional arg + with pytest.warns(DeprecationWarning): + test_func("float32", neutral_arg="test_vector") + + assert len(record) == 1 + assert str(record[0].message) == ( + "Argument old_arg is deprecated and will be removed" + " in the next major release." + ) + + with assert_no_warnings(): + test_func(neutral_arg="test_vector") + + def test_function_positional_argument_required(self): + """ + NOTE: In this situation, it's not possible to avoid a deprecation + warning because the argument is currently required. + """ + + @deprecated_argument("old_arg") + def test_func(old_arg, neutral_arg): + pass + + with pytest.warns(DeprecationWarning): + test_func("float32", "bob") + + def test_function_positional_argument_optional(self): + @deprecated_argument("old_arg") + def test_func(neutral_arg, old_arg=None): pass with pytest.warns(DeprecationWarning): - test_func(dtype="float32") + test_func("bob", "float32") + + with assert_no_warnings(): + test_func("bob") def test_function_keyword_argument(self): - @deprecated_argument("dtype", "vectorizer") - def test_func(dtype=None, vectorizer=None): + @deprecated_argument("old_arg", "new_arg") + def test_func(old_arg=None, new_arg=None): + pass + + with pytest.warns(DeprecationWarning): + test_func(old_arg="float32") + + with assert_no_warnings(): + test_func(new_arg="float32") + + def test_function_keyword_argument_multiple_decorators(self): + @deprecated_argument("old_arg", "new_arg") + @decorator + def test_func(old_arg=None, new_arg=None): pass with pytest.warns(DeprecationWarning): - test_func(vectorizer="float32") + test_func(old_arg="float32") + + with assert_no_warnings(): + test_func(new_arg="float32") + + def test_method_positional_argument_optional(self): + class TestClass: + @deprecated_argument("old_arg", "new_arg") + def test_method(self, new_arg=None, old_arg=None): + pass + + with pytest.warns(DeprecationWarning): + TestClass().test_method("float32", "bob") + + with assert_no_warnings(): + TestClass().test_method("float32") + + def test_method_positional_argument_required(self): + """ + NOTE: In this situation, it's not possible to avoid a deprecation + warning because the argument is currently required. + """ + + class TestClass: + @deprecated_argument("old_arg", "new_arg") + def test_method(self, old_arg, new_arg): + pass + + with pytest.warns(DeprecationWarning): + TestClass().test_method("float32", new_arg="bob") - def test_class_method_argument(self): + def test_method_keyword_argument(self): class TestClass: - @deprecated_argument("dtype", "vectorizer") - def test_method(self, dtype=None, vectorizer=None): + @deprecated_argument("old_arg", "new_arg") + def test_method(self, old_arg=None, new_arg=None): pass with pytest.warns(DeprecationWarning): - TestClass().test_method(dtype="float32") + TestClass().test_method(old_arg="float32") + + with assert_no_warnings(): + TestClass().test_method(new_arg="float32") + + def test_classmethod_positional_argument_required(self): + """ + NOTE: In this situation, it's impossible to avoid a deprecation + warning because the argument is currently required. + """ + + class TestClass: + @deprecated_argument("old_arg", "new_arg") + @classmethod + def test_method(cls, old_arg, new_arg): + pass + + with pytest.warns(DeprecationWarning): + TestClass.test_method("float32", new_arg="bob") + + def test_classmethod_positional_argument_optional(self): + class TestClass: + @deprecated_argument("old_arg", "new_arg") + @classmethod + def test_method(cls, new_arg=None, old_arg=None): + pass + + with pytest.warns(DeprecationWarning): + TestClass.test_method("float32", "bob") + + with assert_no_warnings(): + TestClass.test_method("float32") - def test_class_method_keyword_argument(self): + def test_classmethod_keyword_argument(self): class TestClass: - @deprecated_argument("dtype", "vectorizer") - def test_method(self, dtype=None, vectorizer=None): + @deprecated_argument("old_arg", "new_arg") + @classmethod + def test_method(cls, old_arg=None, new_arg=None): pass with pytest.warns(DeprecationWarning): - TestClass().test_method(vectorizer="float32") + TestClass.test_method(old_arg="float32") + + with assert_no_warnings(): + TestClass.test_method(new_arg="float32") + + def test_classmethod_keyword_argument_multiple_decorators(self): + """ + NOTE: The @deprecated_argument decorator should come between @classmethod + and the method definition. + """ + + class TestClass: + @classmethod + @deprecated_argument("old_arg", "new_arg") + @decorator + def test_method(cls, old_arg=None, new_arg=None): + pass + + with pytest.warns(DeprecationWarning): + TestClass.test_method(old_arg="float32") + + with assert_no_warnings(): + TestClass.test_method(new_arg="float32") def test_class_init_argument(self): class TestClass: - @deprecated_argument("dtype", "vectorizer") - def __init__(self, dtype=None, vectorizer=None): + @deprecated_argument("old_arg", "new_arg") + def __init__(self, old_arg=None, new_arg=None): pass with pytest.warns(DeprecationWarning): - TestClass(dtype="float32") + TestClass(old_arg="float32") def test_class_init_keyword_argument(self): class TestClass: - @deprecated_argument("dtype", "vectorizer") - def __init__(self, dtype=None, vectorizer=None): + @deprecated_argument("old_arg", "new_arg") + def __init__(self, old_arg=None, new_arg=None): + pass + + with pytest.warns(DeprecationWarning): + TestClass(old_arg="float32") + + with assert_no_warnings(): + TestClass(new_arg="float32") + + def test_class_init_keyword_argument_kwargs(self): + class TestClass: + @deprecated_argument("old_arg", "new_arg") + def __init__(self, new_arg=None, **kwargs): pass with pytest.warns(DeprecationWarning): - TestClass(dtype="float32") + TestClass(old_arg="float32") + + with assert_no_warnings(): + TestClass(new_arg="float32") async def test_async_function_argument(self): - @deprecated_argument("dtype", "vectorizer") - async def test_func(dtype=None, vectorizer=None): + @deprecated_argument("old_arg", "new_arg") + async def test_func(old_arg=None, new_arg=None): return 1 with pytest.warns(DeprecationWarning): - result = await test_func(dtype="float32") + result = await test_func(old_arg="float32") assert result == 1 + + async def test_ignores_local_variable(self): + @deprecated_argument("old_arg", "new_arg") + async def test_func(old_arg=None, new_arg=None): + # The presence of this variable should not trigger a warning + old_arg = "float32" + return 1 + + with assert_no_warnings(): + await test_func()