From af2c028a002dd0bce64f34debf4375bd54acc9f1 Mon Sep 17 00:00:00 2001 From: Andrew Brookins Date: Thu, 13 Feb 2025 14:24:55 -0800 Subject: [PATCH 1/3] Avoid looking at local variables for deprecation warnings --- redisvl/utils/utils.py | 3 ++- tests/unit/test_utils.py | 13 +++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/redisvl/utils/utils.py b/redisvl/utils/utils.py index da1067d8..84894df1 100644 --- a/redisvl/utils/utils.py +++ b/redisvl/utils/utils.py @@ -1,3 +1,4 @@ +import inspect import json from enum import Enum from functools import wraps @@ -76,7 +77,7 @@ def deprecated_argument(argument: str, replacement: Optional[str] = None) -> Cal def wrapper(func): @wraps(func) def inner(*args, **kwargs): - argument_names = func.__code__.co_varnames + argument_names = list(inspect.signature(func).parameters.keys()) if argument in argument_names: warn(message, DeprecationWarning, stacklevel=2) diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index cd8a3c9f..390028a5 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -1,3 +1,4 @@ +import warnings import numpy as np import pytest @@ -237,3 +238,15 @@ async def test_func(dtype=None, vectorizer=None): with pytest.warns(DeprecationWarning): result = await test_func(dtype="float32") assert result == 1 + + async def test_ignores_local_variable(self): + @deprecated_argument("dtype") + async def test_func(vectorizer=None): + # The presence of this variable should not trigger a warning + dtype = "float32" + return 1 + + # This will raise an error if any warning is emitted + with warnings.catch_warnings(): + warnings.simplefilter("error") + await test_func() From 2df93a60257048bbefa3ee3d08da498291d68713 Mon Sep 17 00:00:00 2001 From: Andrew Brookins Date: Thu, 13 Feb 2025 16:48:44 -0800 Subject: [PATCH 2/3] Support classmethods --- redisvl/utils/utils.py | 61 ++++++++-- tests/unit/test_utils.py | 232 ++++++++++++++++++++++++++++++++------- 2 files changed, 240 insertions(+), 53 deletions(-) diff --git a/redisvl/utils/utils.py b/redisvl/utils/utils.py index 84894df1..ab9d1c80 100644 --- a/redisvl/utils/utils.py +++ b/redisvl/utils/utils.py @@ -1,5 +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 @@ -68,24 +70,59 @@ 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 = list(inspect.signature(func).parameters.keys()) + 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): + 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): + 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 390028a5..db4fd9d2 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -1,4 +1,6 @@ import warnings +from functools import wraps + import numpy as np import pytest @@ -8,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(): @@ -149,104 +151,252 @@ 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(dtype="float32") + test_func(old_arg="float32") 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." ) + # Test that passing the deprecated argument as a positional argument also triggers the warning. + with pytest.warns(DeprecationWarning) as record: + 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. 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(vectorizer="float32") + 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(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_method_keyword_argument(self): + class TestClass: + @deprecated_argument("old_arg", "new_arg") + def test_method(self, 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_classmethod_positional_argument_required(self): + """ + NOTE: In this situation, it's impossible to avoid a deprecation + warning because the argument is currently required. + """ - def test_class_method_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, new_arg): pass with pytest.warns(DeprecationWarning): - TestClass().test_method(dtype="float32") + TestClass.test_method("float32", new_arg="bob") - def test_class_method_keyword_argument(self): + def test_classmethod_positional_argument_optional(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, new_arg=None, old_arg=None): pass with pytest.warns(DeprecationWarning): - TestClass().test_method(vectorizer="float32") + TestClass.test_method("float32", "bob") + + with assert_no_warnings(): + TestClass.test_method("float32") + + def test_classmethod_keyword_argument(self): + class TestClass: + @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(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(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("dtype") - async def test_func(vectorizer=None): + @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 - dtype = "float32" + old_arg = "float32" return 1 - # This will raise an error if any warning is emitted - with warnings.catch_warnings(): - warnings.simplefilter("error") + with assert_no_warnings(): await test_func() From df1089cb18ef472a8805e1c416133953c442a2a8 Mon Sep 17 00:00:00 2001 From: Andrew Brookins Date: Thu, 13 Feb 2025 17:19:48 -0800 Subject: [PATCH 3/3] support classmethods --- redisvl/utils/utils.py | 18 ++++++++++++------ tests/unit/test_utils.py | 12 ++++++++++++ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/redisvl/utils/utils.py b/redisvl/utils/utils.py index ab9d1c80..164fb2ac 100644 --- a/redisvl/utils/utils.py +++ b/redisvl/utils/utils.py @@ -93,10 +93,13 @@ def decorator(func): @wraps(underlying) def inner_wrapped(*args, **kwargs): - sig = inspect.signature(underlying) - bound_args = sig.bind(*args, **kwargs) - if argument in bound_args.arguments: + 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): @@ -107,10 +110,13 @@ def inner_wrapped(*args, **kwargs): @wraps(func) def inner_normal(*args, **kwargs): - sig = inspect.signature(func) - bound_args = sig.bind(*args, **kwargs) - if argument in bound_args.arguments: + 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 inner_normal diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index db4fd9d2..4be3c556 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -382,6 +382,18 @@ def __init__(self, old_arg=None, new_arg=None): 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(old_arg="float32") + + with assert_no_warnings(): + TestClass(new_arg="float32") + async def test_async_function_argument(self): @deprecated_argument("old_arg", "new_arg") async def test_func(old_arg=None, new_arg=None):