Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(python)!: Enforce deprecation of keyword arguments as positional #16755

Merged
merged 2 commits into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -5358,7 +5357,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 @@ -6849,7 +6847,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")