Skip to content

Commit

Permalink
feat(python)!: Enforce deprecation of keyword arguments as positional (
Browse files Browse the repository at this point in the history
  • Loading branch information
stinodego committed Jun 6, 2024
1 parent b329894 commit 4d35be2
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 86 deletions.
86 changes: 43 additions & 43 deletions py-polars/polars/_utils/deprecation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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]]:
Expand All @@ -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)

Expand Down Expand Up @@ -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:
Expand All @@ -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
3 changes: 0 additions & 3 deletions py-polars/polars/dataframe/frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -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],
Expand Down Expand Up @@ -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:
Expand Down
13 changes: 1 addition & 12 deletions py-polars/polars/functions/lazy.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
3 changes: 0 additions & 3 deletions py-polars/polars/lazyframe/frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -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],
Expand Down Expand Up @@ -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:
Expand Down
4 changes: 3 additions & 1 deletion py-polars/tests/unit/datatypes/test_temporal.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
12 changes: 0 additions & 12 deletions py-polars/tests/unit/operations/test_drop.py
Original file line number Diff line number Diff line change
Expand Up @@ -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())
12 changes: 0 additions & 12 deletions py-polars/tests/unit/operations/test_group_by.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]}
Expand Down
29 changes: 29 additions & 0 deletions py-polars/tests/unit/utils/test_deprecation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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")

0 comments on commit 4d35be2

Please sign in to comment.