From 4d35be233deeed008c1d14b80e838f7bc919e8cb Mon Sep 17 00:00:00 2001 From: Stijn de Gooijer Date: Thu, 6 Jun 2024 13:39:34 +0200 Subject: [PATCH] feat(python)!: Enforce deprecation of keyword arguments as positional (#16755) --- py-polars/polars/_utils/deprecation.py | 86 +++++++++---------- py-polars/polars/dataframe/frame.py | 3 - py-polars/polars/functions/lazy.py | 13 +-- py-polars/polars/lazyframe/frame.py | 3 - .../tests/unit/datatypes/test_temporal.py | 4 +- py-polars/tests/unit/operations/test_drop.py | 12 --- .../tests/unit/operations/test_group_by.py | 12 --- .../tests/unit/utils/test_deprecation.py | 29 +++++++ 8 files changed, 76 insertions(+), 86 deletions(-) diff --git a/py-polars/polars/_utils/deprecation.py b/py-polars/polars/_utils/deprecation.py index d547da5b711c..00c253764804 100644 --- a/py-polars/polars/_utils/deprecation.py +++ b/py-polars/polars/_utils/deprecation.py @@ -53,7 +53,7 @@ def decorate(function: Callable[P, T]) -> Callable[P, T]: @wraps(function) def wrapper(*args: P.args, **kwargs: P.kwargs) -> T: issue_deprecation_warning( - f"`{function.__name__}` is deprecated. {message}", + f"`{function.__qualname__}` is deprecated. {message}", version=version, ) return function(*args, **kwargs) @@ -71,43 +71,6 @@ def deprecate_renamed_function( return deprecate_function(f"It has been renamed to `{new_name}`.", version=version) -def deprecate_parameter_as_positional( - old_name: str, *, version: str -) -> Callable[[Callable[P, T]], Callable[P, T]]: - """ - Decorator to mark a function argument as deprecated due to being made positional. - - Use as follows:: - - @deprecate_parameter_as_positional("column", version="0.20.4") - def myfunc(new_name): ... - """ - - def decorate(function: Callable[P, T]) -> Callable[P, T]: - @wraps(function) - def wrapper(*args: P.args, **kwargs: P.kwargs) -> T: - try: - param_args = kwargs.pop(old_name) - except KeyError: - return function(*args, **kwargs) - - issue_deprecation_warning( - f"named `{old_name}` param is deprecated; use positional `*args` instead.", - version=version, - ) - if not isinstance(param_args, Sequence) or isinstance(param_args, str): - param_args = (param_args,) - elif not isinstance(param_args, tuple): - param_args = tuple(param_args) - args = args + param_args # type: ignore[assignment] - return function(*args, **kwargs) - - wrapper.__signature__ = inspect.signature(function) # type: ignore[attr-defined] - return wrapper - - return decorate - - def deprecate_renamed_parameter( old_name: str, new_name: str, *, version: str ) -> Callable[[Callable[P, T]], Callable[P, T]]: @@ -124,7 +87,7 @@ def decorate(function: Callable[P, T]) -> Callable[P, T]: @wraps(function) def wrapper(*args: P.args, **kwargs: P.kwargs) -> T: _rename_keyword_argument( - old_name, new_name, kwargs, function.__name__, version + old_name, new_name, kwargs, function.__qualname__, version ) return function(*args, **kwargs) @@ -227,10 +190,7 @@ def wrapper(*args: P.args, **kwargs: P.kwargs) -> T: def _format_argument_list(allowed_args: list[str]) -> str: - """ - Format allowed arguments list for use in the warning message of - `deprecate_nonkeyword_arguments`. - """ # noqa: D205 + """Format allowed arguments list for use in the warning message of `deprecate_nonkeyword_arguments`.""" # noqa: W505 if "self" in allowed_args: allowed_args.remove("self") if not allowed_args: @@ -241,3 +201,43 @@ def _format_argument_list(allowed_args: list[str]) -> str: last = allowed_args[-1] args = ", ".join([f"{x!r}" for x in allowed_args[:-1]]) return f" except for {args} and {last!r}" + + +def deprecate_parameter_as_multi_positional( + old_name: str, *, version: str +) -> Callable[[Callable[P, T]], Callable[P, T]]: + """ + Decorator to mark a function argument as deprecated due to being made multi-positional. + + Use as follows:: + + @deprecate_parameter_as_positional("columns", version="0.20.4") + def myfunc(*columns): ... + """ # noqa: W505 + + def decorate(function: Callable[P, T]) -> Callable[P, T]: + @wraps(function) + def wrapper(*args: P.args, **kwargs: P.kwargs) -> T: + try: + arg_value = kwargs.pop(old_name) + except KeyError: + return function(*args, **kwargs) + + issue_deprecation_warning( + f"Passing `{old_name}` as a keyword argument is deprecated." + " Pass it as a positional argument instead.", + version=version, + ) + + if not isinstance(arg_value, Sequence) or isinstance(arg_value, str): + arg_value = (arg_value,) + elif not isinstance(arg_value, tuple): + arg_value = tuple(arg_value) + + args = args + arg_value # type: ignore[assignment] + return function(*args, **kwargs) + + wrapper.__signature__ = inspect.signature(function) # type: ignore[attr-defined] + return wrapper + + return decorate diff --git a/py-polars/polars/dataframe/frame.py b/py-polars/polars/dataframe/frame.py index 15a9f51135e2..ed073cfaa4ed 100644 --- a/py-polars/polars/dataframe/frame.py +++ b/py-polars/polars/dataframe/frame.py @@ -44,7 +44,6 @@ from polars._utils.convert import parse_as_duration_string from polars._utils.deprecation import ( deprecate_function, - deprecate_parameter_as_positional, deprecate_renamed_parameter, issue_deprecation_warning, ) @@ -5381,7 +5380,6 @@ def with_row_count(self, name: str = "row_nr", offset: int = 0) -> Self: """ return self.with_row_index(name, offset) - @deprecate_parameter_as_positional("by", version="0.20.7") def group_by( self, *by: IntoExpr | Iterable[IntoExpr], @@ -6872,7 +6870,6 @@ def extend(self, other: DataFrame) -> Self: raise return self - @deprecate_parameter_as_positional("columns", version="0.20.4") def drop( self, *columns: ColumnNameOrSelector | Iterable[ColumnNameOrSelector] ) -> DataFrame: diff --git a/py-polars/polars/functions/lazy.py b/py-polars/polars/functions/lazy.py index d2a72a557b6b..fa06bf01e1a7 100644 --- a/py-polars/polars/functions/lazy.py +++ b/py-polars/polars/functions/lazy.py @@ -6,10 +6,7 @@ import polars._reexport as pl import polars.functions as F from polars._utils.async_ import _AioDataFrameResult, _GeventDataFrameResult -from polars._utils.deprecation import ( - deprecate_parameter_as_positional, - issue_deprecation_warning, -) +from polars._utils.deprecation import issue_deprecation_warning from polars._utils.parse_expr_input import ( parse_as_expression, parse_as_list_of_expressions, @@ -100,7 +97,6 @@ def element() -> Expr: return F.col("") -@deprecate_parameter_as_positional("column", version="0.20.4") def count(*columns: str) -> Expr: """ Return the number of non-null values in the column. @@ -212,7 +208,6 @@ def cum_count(*columns: str, reverse: bool = False) -> Expr: return F.col(*columns).cum_count(reverse=reverse) -@deprecate_parameter_as_positional("column", version="0.20.4") def implode(*columns: str) -> Expr: """ Aggregate all column values into a list. @@ -334,7 +329,6 @@ def var(column: str, ddof: int = 1) -> Expr: return F.col(column).var(ddof) -@deprecate_parameter_as_positional("column", version="0.20.4") def mean(*columns: str) -> Expr: """ Get the mean value. @@ -382,7 +376,6 @@ def mean(*columns: str) -> Expr: return F.col(*columns).mean() -@deprecate_parameter_as_positional("column", version="0.20.4") def median(*columns: str) -> Expr: """ Get the median value. @@ -426,7 +419,6 @@ def median(*columns: str) -> Expr: return F.col(*columns).median() -@deprecate_parameter_as_positional("column", version="0.20.4") def n_unique(*columns: str) -> Expr: """ Count unique values. @@ -470,7 +462,6 @@ def n_unique(*columns: str) -> Expr: return F.col(*columns).n_unique() -@deprecate_parameter_as_positional("column", version="0.20.4") def approx_n_unique(*columns: str) -> Expr: """ Approximate count of unique values. @@ -515,7 +506,6 @@ def approx_n_unique(*columns: str) -> Expr: return F.col(*columns).approx_n_unique() -@deprecate_parameter_as_positional("column", version="0.20.4") def first(*columns: str) -> Expr: """ Get the first column or value. @@ -581,7 +571,6 @@ def first(*columns: str) -> Expr: return F.col(*columns).first() -@deprecate_parameter_as_positional("column", version="0.20.4") def last(*columns: str) -> Expr: """ Get the last column or value. diff --git a/py-polars/polars/lazyframe/frame.py b/py-polars/polars/lazyframe/frame.py index 120929488e89..ba198093c4df 100644 --- a/py-polars/polars/lazyframe/frame.py +++ b/py-polars/polars/lazyframe/frame.py @@ -28,7 +28,6 @@ from polars._utils.convert import negate_duration_string, parse_as_duration_string from polars._utils.deprecation import ( deprecate_function, - deprecate_parameter_as_positional, deprecate_renamed_parameter, issue_deprecation_warning, ) @@ -3052,7 +3051,6 @@ def select_seq( ) return self._from_pyldf(self._ldf.select_seq(pyexprs)) - @deprecate_parameter_as_positional("by", version="0.20.7") def group_by( self, *by: IntoExpr | Iterable[IntoExpr], @@ -4300,7 +4298,6 @@ def with_context(self, other: Self | list[Self]) -> Self: return self._from_pyldf(self._ldf.with_context([lf._ldf for lf in other])) - @deprecate_parameter_as_positional("columns", version="0.20.4") def drop( self, *columns: ColumnNameOrSelector | Iterable[ColumnNameOrSelector] ) -> Self: diff --git a/py-polars/tests/unit/datatypes/test_temporal.py b/py-polars/tests/unit/datatypes/test_temporal.py index bcfb024b23ee..6c2b82c44fdd 100644 --- a/py-polars/tests/unit/datatypes/test_temporal.py +++ b/py-polars/tests/unit/datatypes/test_temporal.py @@ -434,7 +434,9 @@ def test_to_list() -> None: def test_rows() -> None: s0 = pl.Series("date", [123543, 283478, 1243]).cast(pl.Date) - with pytest.deprecated_call(match="`with_time_unit` is deprecated"): + with pytest.deprecated_call( + match="`ExprDateTimeNameSpace.with_time_unit` is deprecated" + ): s1 = ( pl.Series("datetime", [a * 1_000_000 for a in [123543, 283478, 1243]]) .cast(pl.Datetime) diff --git a/py-polars/tests/unit/operations/test_drop.py b/py-polars/tests/unit/operations/test_drop.py index e7fbc8f9f735..2918ddc26dbf 100644 --- a/py-polars/tests/unit/operations/test_drop.py +++ b/py-polars/tests/unit/operations/test_drop.py @@ -125,15 +125,3 @@ def test_drop_without_parameters() -> None: df = pl.DataFrame({"a": [1, 2]}) assert_frame_equal(df.drop(), df) assert_frame_equal(df.lazy().drop(*[]), df.lazy()) - - -def test_drop_keyword_deprecated() -> None: - df = pl.DataFrame({"a": [1, 2], "b": [3, 4]}) - expected = df.select("b") - with pytest.deprecated_call(): - result_df = df.drop(columns="a") - assert_frame_equal(result_df, expected) - - with pytest.deprecated_call(): - result_lf = df.lazy().drop(columns="a") - assert_frame_equal(result_lf, expected.lazy()) diff --git a/py-polars/tests/unit/operations/test_group_by.py b/py-polars/tests/unit/operations/test_group_by.py index 38d7fb1ceb20..855d867889cb 100644 --- a/py-polars/tests/unit/operations/test_group_by.py +++ b/py-polars/tests/unit/operations/test_group_by.py @@ -873,18 +873,6 @@ def test_group_by_named() -> None: assert_frame_equal(result, expected) -def test_group_by_deprecated_by_arg() -> None: - df = pl.DataFrame({"a": [1, 1, 2, 2, 3, 3], "b": range(6)}) - with pytest.deprecated_call(): - result = df.group_by(by=(pl.col("a") * 2), maintain_order=True).agg( - pl.col("b").min() - ) - expected = df.group_by((pl.col("a") * 2), maintain_order=True).agg( - pl.col("b").min() - ) - assert_frame_equal(result, expected) - - def test_group_by_with_null() -> None: df = pl.DataFrame( {"a": [None, None, None, None], "b": [1, 1, 2, 2], "c": ["x", "y", "z", "u"]} diff --git a/py-polars/tests/unit/utils/test_deprecation.py b/py-polars/tests/unit/utils/test_deprecation.py index 5c067404afd0..c9264164eb0a 100644 --- a/py-polars/tests/unit/utils/test_deprecation.py +++ b/py-polars/tests/unit/utils/test_deprecation.py @@ -8,6 +8,7 @@ from polars._utils.deprecation import ( deprecate_function, deprecate_nonkeyword_arguments, + deprecate_parameter_as_multi_positional, deprecate_renamed_function, deprecate_renamed_parameter, issue_deprecation_warning, @@ -67,3 +68,31 @@ def test_deprecate_nonkeyword_arguments_method_warning() -> None: ) with pytest.deprecated_call(match=msg): Foo().bar("qux", "quox") + + +def test_deprecate_parameter_as_multi_positional(recwarn: Any) -> None: + @deprecate_parameter_as_multi_positional("foo", version="1.0.0") + def hello(*foo: str) -> tuple[str, ...]: + return foo + + with pytest.deprecated_call(): + result = hello(foo="x") + assert result == hello("x") + + with pytest.deprecated_call(): + result = hello(foo=["x", "y"]) # type: ignore[arg-type] + assert result == hello("x", "y") + + +def test_deprecate_parameter_as_multi_positional_existing_arg(recwarn: Any) -> None: + @deprecate_parameter_as_multi_positional("foo", version="1.0.0") + def hello(bar: int, *foo: str) -> tuple[int, tuple[str, ...]]: + return bar, foo + + with pytest.deprecated_call(): + result = hello(5, foo="x") + assert result == hello(5, "x") + + with pytest.deprecated_call(): + result = hello(5, foo=["x", "y"]) # type: ignore[arg-type] + assert result == hello(5, "x", "y")