diff --git a/python/python/lancedb/functions.py b/python/python/lancedb/functions.py index 4bf028306..74cc2adf4 100644 --- a/python/python/lancedb/functions.py +++ b/python/python/lancedb/functions.py @@ -1298,6 +1298,21 @@ def _annotation_type(annotation: Any) -> tuple[pa.DataType, bool]: raise TypeError(f"unsupported Function annotation: {annotation!r}") +def _validate_callable(function: Any) -> None: + if hasattr(function, "__wrapped__"): + raise TypeError( + "decorated wrappers are not supported; put caching inside the function " + "body or use a class Function with a cache in __init__" + ) + if inspect.isgeneratorfunction(function) or inspect.isasyncgenfunction(function): + raise TypeError( + "generator Functions are not supported; return a value instead of " + "yielding, or return a list for multiple values" + ) + if inspect.isfunction(function) and function.__name__ == "": + raise TypeError("lambda Functions are not supported; use a named def instead") + + def _callable_parameters(function: Callable[..., Any]) -> tuple[inspect.Parameter, ...]: parameters = tuple(inspect.signature(function).parameters.values()) for parameter in parameters: @@ -1308,7 +1323,11 @@ def _callable_parameters(function: Callable[..., Any]) -> tuple[inspect.Paramete ): raise TypeError("Function callables require named, non-variadic parameters") if parameter.default is not inspect.Parameter.empty: - raise TypeError("Function callable defaults are not supported") + raise TypeError( + f"Function callable defaults are not supported: {parameter.name!r}; " + "move configuration parameters to a class Function's __init__ " + "and pass them as binding-time initialization values" + ) return parameters @@ -2201,6 +2220,7 @@ def _class_member(cls: type, name: str) -> Any: def _class_call(cls: type) -> Callable[..., Any]: call = _class_member(cls, "__call__") + _validate_callable(call) if not inspect.isfunction(call) or inspect.iscoroutinefunction(call): raise TypeError("a class Function must define a synchronous __call__ method") close = _class_member(cls, "close") @@ -2257,12 +2277,10 @@ class FunctionDefinition: if not is_class and ( not inspect.isfunction(function) or inspect.iscoroutinefunction(function) ): + _validate_callable(function) raise TypeError( "@function requires a synchronous Python function or a class" ) - function_name = name or function.__name__ - if not _FUNCTION_NAME.fullmatch(function_name): - raise ValueError(f"invalid Function name: {function_name!r}") if pip and conda: raise ValueError("a Function environment is pip or conda, not both") if conda_channels and not conda: @@ -2324,6 +2342,11 @@ class FunctionDefinition: "model Functions reserve environment variables " f"{reserved!r}; the worker sets them for the private snapshot" ) + if not is_class: + _validate_callable(function) + function_name = name or function.__name__ + if not _FUNCTION_NAME.fullmatch(function_name): + raise ValueError(f"invalid Function name: {function_name!r}") if is_class: if "<" in function.__qualname__ or "." in function.__qualname__: raise ValueError( @@ -2555,6 +2578,14 @@ def function( reuses them. An instance serves many batches of one binding, so state is shared across them, but not across instances or workers. + Functions and ``__call__`` methods must return a value. Generators, + including async generators, are not supported; return a list to produce + multiple values for a list output. Lambdas and decorated wrappers such as + ``functools.lru_cache`` are not supported. Use a named ``def`` and keep + caches inside the body or in a class instance initialized by ``__init__``. + Callable input parameters cannot have defaults; move configurable values + to class initialization fields instead. + **Initialization.** The annotated parameters of a class's ``__init__`` are the Function's initialization fields. Their values belong to each binding, not to the Function: ``function(text=col("body"), model="small")`` diff --git a/python/python/tests/test_function_packaging.py b/python/python/tests/test_function_packaging.py index 83dfdaeb3..440adc4da 100644 --- a/python/python/tests/test_function_packaging.py +++ b/python/python/tests/test_function_packaging.py @@ -20,7 +20,7 @@ import pyarrow as pa import pytest from lancedb import col -from lancedb.functions import FunctionDefinition, FunctionVersion, function +from lancedb.functions import FunctionDefinition, FunctionVersion, function, udf FIXTURES = ( Path(__file__).parents[3] @@ -127,6 +127,108 @@ def test_function_without_initialization_keeps_its_wire_form(): assert request["artifact"]["kind"] == "python_callable" +@pytest.mark.filterwarnings("ignore:lancedb.udf is deprecated:DeprecationWarning") +@pytest.mark.parametrize("decorator", [function, udf]) +@pytest.mark.parametrize("explicit_schemas", [False, True]) +@pytest.mark.parametrize("class_function", [False, True]) +@pytest.mark.parametrize( + ("declaration", "statement"), + [ + ("def", "yield value"), + ("def", "yield from [value]"), + ("async def", "yield value"), + ], +) +def test_generator_functions_are_rejected_at_decoration( + helper_package, decorator, explicit_schemas, class_function, declaration, statement +): + signature = ( + "__call__(self, value: int)" if class_function else "generates(value: int)" + ) + source = f"{declaration} {signature} -> int:\n {statement}\n" + if class_function: + source = "class generates:\n" + textwrap.indent(source, " ") + module = helper_package("fx_generator", {"__init__.py": source}) + options = ( + { + "input_schema": pa.schema([pa.field("value", pa.int64(), nullable=False)]), + "output_schema": pa.int64(), + } + if explicit_schemas + else {} + ) + with pytest.raises(TypeError) as error: + decorator(**options)(module.generates) + message = str(error.value) + assert "generator Functions are not supported" in message + assert "return a value" in message + assert "return a list" in message + + +@pytest.mark.filterwarnings("ignore:lancedb.udf is deprecated:DeprecationWarning") +@pytest.mark.parametrize("decorator", [function, udf]) +@pytest.mark.parametrize("explicit_name", [False, True]) +def test_lambda_functions_explain_the_named_alternative(decorator, explicit_name): + options = {"name": "double"} if explicit_name else {} + with pytest.raises(TypeError, match="lambda.*use a named def"): + decorator(**options)(lambda value: value * 2) + + +@pytest.mark.parametrize("class_function", [False, True]) +@pytest.mark.parametrize("cache_decorator", ["lru_cache", "cache"]) +def test_cached_functions_explain_where_to_keep_caches( + helper_package, class_function, cache_decorator +): + signature = "__call__(self, value: int)" if class_function else "cached(value: int)" + source = ( + f"@functools.{cache_decorator}\ndef {signature} -> int:\n return value * 2\n" + ) + if class_function: + source = "class cached:\n" + textwrap.indent(source, " ") + module = helper_package("fx_cached", {"__init__.py": "import functools\n" + source}) + with pytest.raises(TypeError) as error: + function(module.cached) + message = str(error.value) + assert "decorated wrappers are not supported" in message + assert "inside the function body" in message + assert "class Function with a cache in __init__" in message + + +@pytest.mark.parametrize("class_function", [False, True]) +@pytest.mark.parametrize("keyword_only", [False, True]) +def test_callable_defaults_explain_class_initialization( + helper_package, class_function, keyword_only +): + parameters = "value: float, " + ("*, " if keyword_only else "") + parameters += "threshold: float = 0.5" + signature = ( + f"__call__(self, {parameters})" + if class_function + else f"thresholded({parameters})" + ) + source = f"def {signature} -> bool:\n return value >= threshold\n" + if class_function: + source = "class thresholded:\n" + textwrap.indent(source, " ") + module = helper_package("fx_defaults", {"__init__.py": source}) + with pytest.raises(TypeError) as error: + function(module.thresholded) + message = str(error.value) + assert "defaults are not supported: 'threshold'" in message + assert "class Function's __init__" in message + assert "binding-time initialization values" in message + + +def test_function_can_return_a_list_from_a_generator_expression(): + @function + def many(value: int) -> list[int]: + return list(item * 2 for item in range(value)) + + namespace = {} + exec(compile(artifact(many), "user_function.py", "exec"), namespace) + assert namespace["many"](3) == [0, 2, 4] + assert many.registration_request.signature.output.arrow_type == "list" + + @pytest.mark.parametrize("kind", ["function", "class"]) @pytest.mark.parametrize( ("global_value", "value_type"),