From 2505ce204d18a6eea77e1f1d2d034e810c13b71f Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Mon, 1 Jul 2024 18:18:36 +0100 Subject: [PATCH 1/8] feat(typing): DRAFT `@deprecated` improvements Supporting draft PR to attach to upcoming issue --- altair/utils/deprecation.py | 152 ++++++++++++++++---------------- altair/vegalite/v5/api.py | 53 ++++------- tests/utils/test_deprecation.py | 20 +++-- tools/update_init_file.py | 2 + 4 files changed, 107 insertions(+), 120 deletions(-) diff --git a/altair/utils/deprecation.py b/altair/utils/deprecation.py index b247e3a4b..af6b3b657 100644 --- a/altair/utils/deprecation.py +++ b/altair/utils/deprecation.py @@ -1,94 +1,94 @@ from __future__ import annotations -import sys -from typing import Callable, TypeVar, TYPE_CHECKING -import warnings -import functools +from typing import TypeVar +from inspect import ismethod +import typing_extensions as te -if sys.version_info >= (3, 10): - from typing import ParamSpec -else: - from typing_extensions import ParamSpec -if TYPE_CHECKING: - from functools import _Wrapped +_T = TypeVar("_T") -T = TypeVar("T") -P = ParamSpec("P") -R = TypeVar("R") +class AltairDeprecationWarning(DeprecationWarning): ... -class AltairDeprecationWarning(UserWarning): - pass +class deprecated(te.deprecated): + """Indicate that a class, function or overload is deprecated. -def deprecated( - message: str | None = None, -) -> Callable[..., type[T] | _Wrapped[P, R, P, R]]: - """Decorator to deprecate a function or class. + When this decorator is applied to an object, the type checker + will generate a diagnostic on usage of the deprecated object. Parameters ---------- - message : string (optional) - The deprecation message + message + Additional message appended to ``version``, ``alternative``. + version + ``altair`` version the deprecation first appeared. + alternative + Suggested replacement class/method/function. + category + If the *category* is ``None``, no warning is emitted at runtime. + stacklevel + The *stacklevel* determines where the + warning is emitted. If it is ``1`` (the default), the warning + is emitted at the direct caller of the deprecated object; if it + is higher, it is emitted further up the stack. + Static type checker behavior is not affected by the *category* + and *stacklevel* arguments. """ - def wrapper(obj: type[T] | Callable[P, R]) -> type[T] | _Wrapped[P, R, P, R]: - return _deprecate(obj, message=message) - - return wrapper - - -def _deprecate( - obj: type[T] | Callable[P, R], name: str | None = None, message: str | None = None -) -> type[T] | _Wrapped[P, R, P, R]: - """Return a version of a class or function that raises a deprecation warning. + def __init__( + self, + message: te.LiteralString, + /, + *, + version: str, + alternative: str | None = None, + category: type[AltairDeprecationWarning] | None = AltairDeprecationWarning, + stacklevel: int = 1, + ) -> None: + super().__init__(message, category=category, stacklevel=stacklevel) + self.version = version + self.alternative = alternative + + def __call__(self, arg: _T, /) -> _T: + if name := getattr(arg, "__name__"): # noqa: B009 + # HACK: + # - The annotation for `arg` is required for `mypy` to be happy + # - The attribute check is for `pyright` + ... + else: + msg = ( + f"{type(self).__qualname__!r} can only be used on" + f" types with a '__name__' attribute, and {arg!r} does not.\n\n" + "See https://docs.python.org/3/reference/datamodel.html#callable-types" + ) + raise TypeError(msg) + msg = f"alt.{name} was deprecated in `altair={self.version}`." + if self.alternative: + prefix = arg.__qualname__.split(".")[0] if ismethod(arg) else "alt" + msg = f"{msg} Use {prefix}.{self.alternative} instead." + self.message = f"{msg}\n\n{self.message}" if self.message else msg # pyright: ignore[reportAttributeAccessIssue] + return super().__call__(arg) + + +def msg( + *, + version: te.LiteralString, + alternative: te.LiteralString | None = None, + message: te.LiteralString | None = None, +) -> te.LiteralString: + """Generate a standard deprecation message. Parameters ---------- - obj : class or function - The object to create a deprecated version of. - name : string (optional) - The name of the deprecated object - message : string (optional) - The deprecation message - - Returns - ------- - deprecated_obj : - The deprecated version of obj - - Examples - -------- - >>> class Foo: pass - >>> OldFoo = _deprecate(Foo, "OldFoo") - >>> f = OldFoo() # doctest: +SKIP - AltairDeprecationWarning: alt.OldFoo is deprecated. Use alt.Foo instead. + version + ``altair`` version the deprecation first appeared. + alternative + Suggested replacement class/method/function. + message + Additional message appended to ``version``, ``alternative``. """ - if message is None: - message = f"alt.{name} is deprecated. Use alt.{obj.__name__} instead." "" - if isinstance(obj, type): - if name is None: - msg = f"Requires name, but got: {name=}" - raise TypeError(msg) - else: - return type( - name, - (obj,), - { - "__doc__": obj.__doc__, - "__init__": _deprecate(obj.__init__, "__init__", message), - }, - ) - elif callable(obj): - - @functools.wraps(obj) - def new_obj(*args: P.args, **kwargs: P.kwargs) -> R: - warnings.warn(message, AltairDeprecationWarning, stacklevel=1) - return obj(*args, **kwargs) - - new_obj._deprecated = True # type: ignore[attr-defined] - return new_obj - else: - msg = f"Cannot deprecate object of type {type(obj)}" - raise ValueError(msg) + output = f"Deprecated in `altair={version}`." + if alternative: + output = f"{output} Use {alternative} instead." + return f"{output}\n\n{message}" if message else output diff --git a/altair/vegalite/v5/api.py b/altair/vegalite/v5/api.py index f25491865..e82271941 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -7,7 +7,7 @@ import jsonschema import itertools from typing import Union, cast, Any, Iterable, Literal, IO, TYPE_CHECKING -from typing_extensions import TypeAlias +from typing_extensions import TypeAlias, deprecated import typing from .schema import core, channels, mixins, Undefined, SCHEMA_URL @@ -263,7 +263,7 @@ def __init__( self.param_type = param_type @utils.deprecation.deprecated( - message="'ref' is deprecated. No need to call '.ref()' anymore." + "No need to call '.ref()' anymore.", version="5.0.0", alternative="to_dict" ) def ref(self) -> dict: "'ref' is deprecated. No need to call '.ref()' anymore." @@ -533,9 +533,12 @@ def _selection( return param(select=select, **param_kwds) -@utils.deprecation.deprecated( - message="""'selection' is deprecated. - Use 'selection_point()' or 'selection_interval()' instead; these functions also include more helpful docstrings.""" +@deprecated( + utils.deprecation.msg( + version="5.0.0", + alternative="'selection_point()' or 'selection_interval()'", + message="These functions also include more helpful docstrings.", + ) ) def selection( type: Optional[Literal["interval", "point"]] = Undefined, **kwds @@ -790,19 +793,13 @@ def selection_point( ) -@utils.deprecation.deprecated( - message="'selection_multi' is deprecated. Use 'selection_point'" -) -@utils.use_signature(core.PointSelectionConfig) +@utils.deprecation.deprecated("", version="5.0.0", alternative="selection_point") def selection_multi(**kwargs): """'selection_multi' is deprecated. Use 'selection_point'""" return _selection(type="point", **kwargs) -@utils.deprecation.deprecated( - message="'selection_single' is deprecated. Use 'selection_point'" -) -@utils.use_signature(core.PointSelectionConfig) +@utils.deprecation.deprecated("", version="5.0.0", alternative="selection_point") def selection_single(**kwargs): """'selection_single' is deprecated. Use 'selection_point'""" return _selection(type="point", **kwargs) @@ -2659,7 +2656,7 @@ def display( else: display(self) - @utils.deprecation.deprecated(message="'serve' is deprecated. Use 'show' instead.") + @utils.deprecation.deprecated("", version="4.1.0", alternative="show") def serve( self, ip="127.0.0.1", @@ -3033,9 +3030,7 @@ def add_params(self, *params: Parameter) -> Self: copy.params.append(s.param) return copy - @utils.deprecation.deprecated( - message="'add_selection' is deprecated. Use 'add_params' instead." - ) + @utils.deprecation.deprecated("", version="5.0.0", alternative="add_params") def add_selection(self, *params) -> Self: """'add_selection' is deprecated. Use 'add_params' instead.""" return self.add_params(*params) @@ -3249,9 +3244,7 @@ def add_params(self, *params: Parameter) -> Self: copy.spec = copy.spec.add_params(*params) return copy.copy() - @utils.deprecation.deprecated( - message="'add_selection' is deprecated. Use 'add_params' instead." - ) + @utils.deprecation.deprecated("", version="5.0.0", alternative="add_params") def add_selection(self, *selections) -> Self: """'add_selection' is deprecated. Use 'add_params' instead.""" return self.add_params(*selections) @@ -3366,9 +3359,7 @@ def add_params(self, *params: Parameter) -> Self: copy.concat = [chart.add_params(*params) for chart in copy.concat] return copy - @utils.deprecation.deprecated( - message="'add_selection' is deprecated. Use 'add_params' instead." - ) + @utils.deprecation.deprecated("", version="5.0.0", alternative="add_params") def add_selection(self, *selections) -> Self: """'add_selection' is deprecated. Use 'add_params' instead.""" return self.add_params(*selections) @@ -3463,9 +3454,7 @@ def add_params(self, *params: Parameter) -> Self: copy.hconcat = [chart.add_params(*params) for chart in copy.hconcat] return copy - @utils.deprecation.deprecated( - message="'add_selection' is deprecated. Use 'add_params' instead." - ) + @utils.deprecation.deprecated("", version="5.0.0", alternative="add_params") def add_selection(self, *selections) -> Self: """'add_selection' is deprecated. Use 'add_params' instead.""" return self.add_params(*selections) @@ -3562,9 +3551,7 @@ def add_params(self, *params: Parameter) -> Self: copy.vconcat = [chart.add_params(*params) for chart in copy.vconcat] return copy - @utils.deprecation.deprecated( - message="'add_selection' is deprecated. Use 'add_params' instead." - ) + @utils.deprecation.deprecated("", version="5.0.0", alternative="add_params") def add_selection(self, *selections) -> Self: """'add_selection' is deprecated. Use 'add_params' instead.""" return self.add_params(*selections) @@ -3681,9 +3668,7 @@ def add_params(self, *params: Parameter) -> Self: copy.layer[0] = copy.layer[0].add_params(*params) return copy.copy() - @utils.deprecation.deprecated( - message="'add_selection' is deprecated. Use 'add_params' instead." - ) + @utils.deprecation.deprecated("", version="5.0.0", alternative="add_params") def add_selection(self, *selections) -> Self: """'add_selection' is deprecated. Use 'add_params' instead.""" return self.add_params(*selections) @@ -3769,9 +3754,7 @@ def add_params(self, *params: Parameter) -> Self: copy.spec = copy.spec.add_params(*params) return copy.copy() - @utils.deprecation.deprecated( - message="'add_selection' is deprecated. Use 'add_params' instead." - ) + @utils.deprecation.deprecated("", version="5.0.0", alternative="add_params") def add_selection(self, *selections) -> Self: """'add_selection' is deprecated. Use 'add_params' instead.""" return self.add_params(*selections) diff --git a/tests/utils/test_deprecation.py b/tests/utils/test_deprecation.py index 4754fb187..4594f75a1 100644 --- a/tests/utils/test_deprecation.py +++ b/tests/utils/test_deprecation.py @@ -1,24 +1,26 @@ import pytest -import altair as alt from altair.utils import AltairDeprecationWarning -from altair.utils.deprecation import _deprecate, deprecated +from altair.utils.deprecation import deprecated def test_deprecated_class(): - OldChart = _deprecate(alt.Chart, "OldChart") - with pytest.warns(AltairDeprecationWarning) as record: + class Dummy: + def __init__(self, *args) -> None: + self.args = args + + OldChart = deprecated("", version="", alternative="LayerChart")(Dummy) + with pytest.warns(AltairDeprecationWarning, match=r"alt\.Dummy.+alt\.LayerChart"): OldChart() - assert "alt.OldChart" in record[0].message.args[0] - assert "alt.Chart" in record[0].message.args[0] def test_deprecation_decorator(): - @deprecated(message="func is deprecated") + @deprecated("", version="999", alternative="12345") def func(x): return x + 1 - with pytest.warns(AltairDeprecationWarning) as record: + with pytest.warns( + AltairDeprecationWarning, match=r"alt\.func.+altair=999.+12345 instead" + ): y = func(1) assert y == 2 - assert record[0].message.args[0] == "func is deprecated" diff --git a/tools/update_init_file.py b/tools/update_init_file.py index 5e8bbc6ab..28ba165f0 100644 --- a/tools/update_init_file.py +++ b/tools/update_init_file.py @@ -38,6 +38,7 @@ Sequence, IO, annotations, + te.deprecated, } @@ -112,6 +113,7 @@ def _is_relevant(attr: Any, name: str, /) -> bool: or attr is TYPE_CHECKING or (_is_hashable(attr) and attr in _TYPING_CONSTRUCTS) or name in {"pd", "jsonschema"} + or getattr_static(attr, "__deprecated__", False) ): return False elif ismodule(attr): From fe1917e3d4986dfa431f428146819622b83b6546 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Mon, 1 Jul 2024 19:10:31 +0100 Subject: [PATCH 2/8] feat: adds `deprecate` function This wrapper format seems to satisfy `pyright` enough to trigger a strikethrough --- altair/utils/deprecation.py | 42 +++++++++++++++++++++++++++++++++++++ altair/vegalite/v5/api.py | 2 +- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/altair/utils/deprecation.py b/altair/utils/deprecation.py index af6b3b657..17f412bb8 100644 --- a/altair/utils/deprecation.py +++ b/altair/utils/deprecation.py @@ -71,6 +71,48 @@ def __call__(self, arg: _T, /) -> _T: return super().__call__(arg) +# NOTE: Annotating the return type breaks `pyright` detecting [reportDeprecated] +def deprecate( + *, + version: te.LiteralString, + alternative: te.LiteralString | None = None, + message: te.LiteralString | None = None, + category: type[AltairDeprecationWarning] | None = AltairDeprecationWarning, + stacklevel: int = 1, +): + """Indicate that a class, function or overload is deprecated. + + When this decorator is applied to an object, the type checker + will generate a diagnostic on usage of the deprecated object. + + Parameters + ---------- + version + ``altair`` version the deprecation first appeared. + alternative + Suggested replacement class/method/function. + message + Additional message appended to ``version``, ``alternative``. + category + If the *category* is ``None``, no warning is emitted at runtime. + stacklevel + The *stacklevel* determines where the + warning is emitted. If it is ``1`` (the default), the warning + is emitted at the direct caller of the deprecated object; if it + is higher, it is emitted further up the stack. + Static type checker behavior is not affected by the *category* + and *stacklevel* arguments. + """ + output = f"Deprecated in `altair={version}`." + if alternative: + output = f"{output} Use {alternative} instead." + return te.deprecated( + f"{output}\n\n{message}" if message else output, + category=category, + stacklevel=stacklevel, + ) + + def msg( *, version: te.LiteralString, diff --git a/altair/vegalite/v5/api.py b/altair/vegalite/v5/api.py index e82271941..7c0d2ac76 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -799,7 +799,7 @@ def selection_multi(**kwargs): return _selection(type="point", **kwargs) -@utils.deprecation.deprecated("", version="5.0.0", alternative="selection_point") +@utils.deprecation.deprecate(version="5.0.0", alternative="selection_point") def selection_single(**kwargs): """'selection_single' is deprecated. Use 'selection_point'""" return _selection(type="point", **kwargs) From 4e7ee2ac240401b4207e342e1b31ce32f4b9958e Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Wed, 3 Jul 2024 14:40:14 +0100 Subject: [PATCH 3/8] refactor: use singular `@deprecated`, export in `utils` - The export is safe and doesn't leak to top level --- altair/utils/__init__.py | 3 +- altair/utils/deprecation.py | 112 ++++++-------------------------- altair/vegalite/v5/api.py | 36 +++++----- tests/utils/test_deprecation.py | 12 ++-- 4 files changed, 43 insertions(+), 120 deletions(-) diff --git a/altair/utils/__init__.py b/altair/utils/__init__.py index 64d6f4566..97fd67f26 100644 --- a/altair/utils/__init__.py +++ b/altair/utils/__init__.py @@ -11,7 +11,7 @@ ) from .html import spec_to_html from .plugin_registry import PluginRegistry -from .deprecation import AltairDeprecationWarning +from .deprecation import AltairDeprecationWarning, deprecated from .schemapi import Undefined, Optional @@ -21,6 +21,7 @@ "PluginRegistry", "SchemaBase", "Undefined", + "deprecated", "display_traceback", "infer_encoding_types", "infer_vegalite_type", diff --git a/altair/utils/deprecation.py b/altair/utils/deprecation.py index 17f412bb8..131214569 100644 --- a/altair/utils/deprecation.py +++ b/altair/utils/deprecation.py @@ -1,85 +1,34 @@ from __future__ import annotations -from typing import TypeVar -from inspect import ismethod -import typing_extensions as te +import sys +from typing import TYPE_CHECKING +if sys.version_info >= (3, 13): + from warnings import deprecated as _deprecated +else: + from typing_extensions import deprecated as _deprecated -_T = TypeVar("_T") +if TYPE_CHECKING: + if sys.version_info >= (3, 11): + from typing import LiteralString + else: + from typing_extensions import LiteralString -class AltairDeprecationWarning(DeprecationWarning): ... - - -class deprecated(te.deprecated): - """Indicate that a class, function or overload is deprecated. - - When this decorator is applied to an object, the type checker - will generate a diagnostic on usage of the deprecated object. - - Parameters - ---------- - message - Additional message appended to ``version``, ``alternative``. - version - ``altair`` version the deprecation first appeared. - alternative - Suggested replacement class/method/function. - category - If the *category* is ``None``, no warning is emitted at runtime. - stacklevel - The *stacklevel* determines where the - warning is emitted. If it is ``1`` (the default), the warning - is emitted at the direct caller of the deprecated object; if it - is higher, it is emitted further up the stack. - Static type checker behavior is not affected by the *category* - and *stacklevel* arguments. - """ - - def __init__( - self, - message: te.LiteralString, - /, - *, - version: str, - alternative: str | None = None, - category: type[AltairDeprecationWarning] | None = AltairDeprecationWarning, - stacklevel: int = 1, - ) -> None: - super().__init__(message, category=category, stacklevel=stacklevel) - self.version = version - self.alternative = alternative - def __call__(self, arg: _T, /) -> _T: - if name := getattr(arg, "__name__"): # noqa: B009 - # HACK: - # - The annotation for `arg` is required for `mypy` to be happy - # - The attribute check is for `pyright` - ... - else: - msg = ( - f"{type(self).__qualname__!r} can only be used on" - f" types with a '__name__' attribute, and {arg!r} does not.\n\n" - "See https://docs.python.org/3/reference/datamodel.html#callable-types" - ) - raise TypeError(msg) - msg = f"alt.{name} was deprecated in `altair={self.version}`." - if self.alternative: - prefix = arg.__qualname__.split(".")[0] if ismethod(arg) else "alt" - msg = f"{msg} Use {prefix}.{self.alternative} instead." - self.message = f"{msg}\n\n{self.message}" if self.message else msg # pyright: ignore[reportAttributeAccessIssue] - return super().__call__(arg) +class AltairDeprecationWarning(DeprecationWarning): ... # NOTE: Annotating the return type breaks `pyright` detecting [reportDeprecated] -def deprecate( +# NOTE: `LiteralString` requirement is introduced by stubs +def deprecated( *, - version: te.LiteralString, - alternative: te.LiteralString | None = None, - message: te.LiteralString | None = None, + version: LiteralString, + alternative: LiteralString | None = None, + message: LiteralString | None = None, category: type[AltairDeprecationWarning] | None = AltairDeprecationWarning, stacklevel: int = 1, -): +): # te.deprecated """Indicate that a class, function or overload is deprecated. When this decorator is applied to an object, the type checker @@ -106,31 +55,8 @@ def deprecate( output = f"Deprecated in `altair={version}`." if alternative: output = f"{output} Use {alternative} instead." - return te.deprecated( + return _deprecated( f"{output}\n\n{message}" if message else output, category=category, stacklevel=stacklevel, ) - - -def msg( - *, - version: te.LiteralString, - alternative: te.LiteralString | None = None, - message: te.LiteralString | None = None, -) -> te.LiteralString: - """Generate a standard deprecation message. - - Parameters - ---------- - version - ``altair`` version the deprecation first appeared. - alternative - Suggested replacement class/method/function. - message - Additional message appended to ``version``, ``alternative``. - """ - output = f"Deprecated in `altair={version}`." - if alternative: - output = f"{output} Use {alternative} instead." - return f"{output}\n\n{message}" if message else output diff --git a/altair/vegalite/v5/api.py b/altair/vegalite/v5/api.py index 7c0d2ac76..3ae84b536 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -7,7 +7,7 @@ import jsonschema import itertools from typing import Union, cast, Any, Iterable, Literal, IO, TYPE_CHECKING -from typing_extensions import TypeAlias, deprecated +from typing_extensions import TypeAlias import typing from .schema import core, channels, mixins, Undefined, SCHEMA_URL @@ -262,9 +262,7 @@ def __init__( self.param = param self.param_type = param_type - @utils.deprecation.deprecated( - "No need to call '.ref()' anymore.", version="5.0.0", alternative="to_dict" - ) + @utils.deprecated(version="5.0.0", alternative="to_dict") def ref(self) -> dict: "'ref' is deprecated. No need to call '.ref()' anymore." return self.to_dict() @@ -533,12 +531,10 @@ def _selection( return param(select=select, **param_kwds) -@deprecated( - utils.deprecation.msg( - version="5.0.0", - alternative="'selection_point()' or 'selection_interval()'", - message="These functions also include more helpful docstrings.", - ) +@utils.deprecated( + version="5.0.0", + alternative="'selection_point()' or 'selection_interval()'", + message="These functions also include more helpful docstrings.", ) def selection( type: Optional[Literal["interval", "point"]] = Undefined, **kwds @@ -793,13 +789,13 @@ def selection_point( ) -@utils.deprecation.deprecated("", version="5.0.0", alternative="selection_point") +@utils.deprecated(version="5.0.0", alternative="selection_point") def selection_multi(**kwargs): """'selection_multi' is deprecated. Use 'selection_point'""" return _selection(type="point", **kwargs) -@utils.deprecation.deprecate(version="5.0.0", alternative="selection_point") +@utils.deprecated(version="5.0.0", alternative="selection_point") def selection_single(**kwargs): """'selection_single' is deprecated. Use 'selection_point'""" return _selection(type="point", **kwargs) @@ -2656,7 +2652,7 @@ def display( else: display(self) - @utils.deprecation.deprecated("", version="4.1.0", alternative="show") + @utils.deprecated(version="4.1.0", alternative="show") def serve( self, ip="127.0.0.1", @@ -3030,7 +3026,7 @@ def add_params(self, *params: Parameter) -> Self: copy.params.append(s.param) return copy - @utils.deprecation.deprecated("", version="5.0.0", alternative="add_params") + @utils.deprecated(version="5.0.0", alternative="add_params") def add_selection(self, *params) -> Self: """'add_selection' is deprecated. Use 'add_params' instead.""" return self.add_params(*params) @@ -3244,7 +3240,7 @@ def add_params(self, *params: Parameter) -> Self: copy.spec = copy.spec.add_params(*params) return copy.copy() - @utils.deprecation.deprecated("", version="5.0.0", alternative="add_params") + @utils.deprecated(version="5.0.0", alternative="add_params") def add_selection(self, *selections) -> Self: """'add_selection' is deprecated. Use 'add_params' instead.""" return self.add_params(*selections) @@ -3359,7 +3355,7 @@ def add_params(self, *params: Parameter) -> Self: copy.concat = [chart.add_params(*params) for chart in copy.concat] return copy - @utils.deprecation.deprecated("", version="5.0.0", alternative="add_params") + @utils.deprecated(version="5.0.0", alternative="add_params") def add_selection(self, *selections) -> Self: """'add_selection' is deprecated. Use 'add_params' instead.""" return self.add_params(*selections) @@ -3454,7 +3450,7 @@ def add_params(self, *params: Parameter) -> Self: copy.hconcat = [chart.add_params(*params) for chart in copy.hconcat] return copy - @utils.deprecation.deprecated("", version="5.0.0", alternative="add_params") + @utils.deprecated(version="5.0.0", alternative="add_params") def add_selection(self, *selections) -> Self: """'add_selection' is deprecated. Use 'add_params' instead.""" return self.add_params(*selections) @@ -3551,7 +3547,7 @@ def add_params(self, *params: Parameter) -> Self: copy.vconcat = [chart.add_params(*params) for chart in copy.vconcat] return copy - @utils.deprecation.deprecated("", version="5.0.0", alternative="add_params") + @utils.deprecated(version="5.0.0", alternative="add_params") def add_selection(self, *selections) -> Self: """'add_selection' is deprecated. Use 'add_params' instead.""" return self.add_params(*selections) @@ -3668,7 +3664,7 @@ def add_params(self, *params: Parameter) -> Self: copy.layer[0] = copy.layer[0].add_params(*params) return copy.copy() - @utils.deprecation.deprecated("", version="5.0.0", alternative="add_params") + @utils.deprecated(version="5.0.0", alternative="add_params") def add_selection(self, *selections) -> Self: """'add_selection' is deprecated. Use 'add_params' instead.""" return self.add_params(*selections) @@ -3754,7 +3750,7 @@ def add_params(self, *params: Parameter) -> Self: copy.spec = copy.spec.add_params(*params) return copy.copy() - @utils.deprecation.deprecated("", version="5.0.0", alternative="add_params") + @utils.deprecated(version="5.0.0", alternative="add_params") def add_selection(self, *selections) -> Self: """'add_selection' is deprecated. Use 'add_params' instead.""" return self.add_params(*selections) diff --git a/tests/utils/test_deprecation.py b/tests/utils/test_deprecation.py index 4594f75a1..ac3b4a4bc 100644 --- a/tests/utils/test_deprecation.py +++ b/tests/utils/test_deprecation.py @@ -1,7 +1,6 @@ import pytest -from altair.utils import AltairDeprecationWarning -from altair.utils.deprecation import deprecated +from altair.utils.deprecation import AltairDeprecationWarning, deprecated def test_deprecated_class(): @@ -9,18 +8,19 @@ class Dummy: def __init__(self, *args) -> None: self.args = args - OldChart = deprecated("", version="", alternative="LayerChart")(Dummy) - with pytest.warns(AltairDeprecationWarning, match=r"alt\.Dummy.+alt\.LayerChart"): + OldChart = deprecated(version="2.0.0", alternative="LayerChart")(Dummy) + + with pytest.warns(AltairDeprecationWarning, match=r"altair=2\.0\.0.+LayerChart"): OldChart() def test_deprecation_decorator(): - @deprecated("", version="999", alternative="12345") + @deprecated(version="999", alternative="func_12345") def func(x): return x + 1 with pytest.warns( - AltairDeprecationWarning, match=r"alt\.func.+altair=999.+12345 instead" + AltairDeprecationWarning, match=r"altair=999.+func_12345 instead" ): y = func(1) assert y == 2 From 4b6699b0e6e734b41ed54638c375f6d8d4010ddb Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Wed, 3 Jul 2024 19:05:20 +0100 Subject: [PATCH 4/8] refactor: extract `_format_message`, add `PEP 702` reference --- altair/utils/deprecation.py | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/altair/utils/deprecation.py b/altair/utils/deprecation.py index 131214569..740920d35 100644 --- a/altair/utils/deprecation.py +++ b/altair/utils/deprecation.py @@ -19,6 +19,18 @@ class AltairDeprecationWarning(DeprecationWarning): ... +def _format_message( + version: LiteralString, + alternative: LiteralString | None, + message: LiteralString | None, + /, +) -> LiteralString: + output = f"Deprecated in `altair={version}`." + if alternative: + output = f"{output} Use {alternative} instead." + return f"{output}\n{message}" if message else output + + # NOTE: Annotating the return type breaks `pyright` detecting [reportDeprecated] # NOTE: `LiteralString` requirement is introduced by stubs def deprecated( @@ -51,12 +63,10 @@ def deprecated( is higher, it is emitted further up the stack. Static type checker behavior is not affected by the *category* and *stacklevel* arguments. + + References + ---------- + [PEP 702](https://peps.python.org/pep-0702/) """ - output = f"Deprecated in `altair={version}`." - if alternative: - output = f"{output} Use {alternative} instead." - return _deprecated( - f"{output}\n\n{message}" if message else output, - category=category, - stacklevel=stacklevel, - ) + msg = _format_message(version, alternative, message) + return _deprecated(msg, category=category, stacklevel=stacklevel) From 08f540849d4c86672c822d93a5a114cb266cbfa8 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Wed, 3 Jul 2024 19:06:52 +0100 Subject: [PATCH 5/8] feat: adds `deprecated_warn` Non-decorator counterpart to `@deprecated` --- altair/utils/__init__.py | 3 ++- altair/utils/deprecation.py | 41 +++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/altair/utils/__init__.py b/altair/utils/__init__.py index 97fd67f26..28650911e 100644 --- a/altair/utils/__init__.py +++ b/altair/utils/__init__.py @@ -11,7 +11,7 @@ ) from .html import spec_to_html from .plugin_registry import PluginRegistry -from .deprecation import AltairDeprecationWarning, deprecated +from .deprecation import AltairDeprecationWarning, deprecated, deprecated_warn from .schemapi import Undefined, Optional @@ -22,6 +22,7 @@ "SchemaBase", "Undefined", "deprecated", + "deprecated_warn", "display_traceback", "infer_encoding_types", "infer_vegalite_type", diff --git a/altair/utils/deprecation.py b/altair/utils/deprecation.py index 740920d35..88c12516c 100644 --- a/altair/utils/deprecation.py +++ b/altair/utils/deprecation.py @@ -1,6 +1,7 @@ from __future__ import annotations import sys +import warnings from typing import TYPE_CHECKING if sys.version_info >= (3, 13): @@ -70,3 +71,43 @@ def deprecated( """ msg = _format_message(version, alternative, message) return _deprecated(msg, category=category, stacklevel=stacklevel) + + +def deprecated_warn( + message: LiteralString, + *, + version: LiteralString, + alternative: LiteralString | None = None, + category: type[AltairDeprecationWarning] = AltairDeprecationWarning, + stacklevel: int = 2, +) -> None: + """Indicate that the current code path is deprecated. + + This should be used for non-trivial cases *only*. ``@deprecated`` should + always be preferred as it is recognized by static type checkers. + + Parameters + ---------- + message + Explanation of the deprecated behaviour. + + .. note:: + Unlike ``@deprecated``, this is *not* optional. + + version + ``altair`` version the deprecation first appeared. + alternative + Suggested replacement argument/method/function. + category + The runtime warning type emitted. + stacklevel + How far up the call stack to make this warning appear. + A value of ``2`` attributes the warning to the caller + of the code calling ``deprecated_warn()``. + + References + ---------- + [warnings.warn](https://docs.python.org/3/library/warnings.html#warnings.warn) + """ + msg = _format_message(version, alternative, message) + warnings.warn(msg, category=category, stacklevel=stacklevel) From bf32c143944e4187c5bdc9e08256c6663d972d29 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Wed, 3 Jul 2024 19:07:23 +0100 Subject: [PATCH 6/8] test: add test for `deprecated_warn` --- tests/utils/test_deprecation.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tests/utils/test_deprecation.py b/tests/utils/test_deprecation.py index ac3b4a4bc..205652292 100644 --- a/tests/utils/test_deprecation.py +++ b/tests/utils/test_deprecation.py @@ -1,6 +1,10 @@ import pytest - -from altair.utils.deprecation import AltairDeprecationWarning, deprecated +import re +from altair.utils.deprecation import ( + AltairDeprecationWarning, + deprecated, + deprecated_warn, +) def test_deprecated_class(): @@ -24,3 +28,11 @@ def func(x): ): y = func(1) assert y == 2 + + +def test_deprecation_warn(): + with pytest.warns( + AltairDeprecationWarning, + match=re.compile(r"altair=3321.+this code path is a noop", flags=re.DOTALL), + ): + deprecated_warn("this code path is a noop", version="3321", stacklevel=1) From 90f4ffcb438ad55535c2d6e7df3e4d8075f782b8 Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Wed, 3 Jul 2024 19:10:45 +0100 Subject: [PATCH 7/8] refactor: standardise warnings with `deprecated_warn` ] --- altair/utils/save.py | 12 +++----- altair/vegalite/v5/api.py | 64 ++++++++++++++------------------------- 2 files changed, 27 insertions(+), 49 deletions(-) diff --git a/altair/utils/save.py b/altair/utils/save.py index 3a8816e17..61d13d8d1 100644 --- a/altair/utils/save.py +++ b/altair/utils/save.py @@ -7,7 +7,7 @@ from .mimebundle import spec_to_mimebundle from ..vegalite.v5.data import data_transformers from altair.utils._vegafusion_data import using_vegafusion -from altair.utils.deprecation import AltairDeprecationWarning +from altair.utils.deprecation import deprecated_warn if TYPE_CHECKING: from pathlib import Path @@ -135,12 +135,10 @@ def save( additional kwargs passed to spec_to_mimebundle. """ if webdriver is not None: - warnings.warn( - "The webdriver argument is deprecated as it's not relevant for" - + " the new vl-convert engine which replaced altair_saver." - + " The argument will be removed in a future release.", - AltairDeprecationWarning, - stacklevel=1, + deprecated_warn( + "The webdriver argument is not relevant for the new vl-convert engine which replaced altair_saver. " + "The argument will be removed in a future release.", + version="5.0.0", ) if json_kwds is None: diff --git a/altair/vegalite/v5/api.py b/altair/vegalite/v5/api.py index 3ae84b536..ec3c06f11 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -23,8 +23,8 @@ using_vegafusion as _using_vegafusion, compile_with_vegafusion as _compile_with_vegafusion, ) -from ...utils.data import DataType, is_data_type as _is_data_type -from ...utils.deprecation import AltairDeprecationWarning +from altair.utils.data import DataType, is_data_type as _is_data_type +from altair.utils.deprecation import AltairDeprecationWarning # noqa: F401 if TYPE_CHECKING: from ...utils.core import DataFrameLike @@ -443,41 +443,24 @@ def param( parameter: Parameter The parameter object that can be used in chart creation. """ + warn_msg = "The value of `empty` should be True or False." + empty_remap = {"none": False, "all": True} parameter = Parameter(name) if empty is not Undefined: - parameter.empty = empty - if parameter.empty == "none": - warnings.warn( - """The value of 'empty' should be True or False.""", - utils.AltairDeprecationWarning, - stacklevel=1, - ) - parameter.empty = False - elif parameter.empty == "all": - warnings.warn( - """The value of 'empty' should be True or False.""", - utils.AltairDeprecationWarning, - stacklevel=1, - ) - parameter.empty = True - elif (parameter.empty is False) or (parameter.empty is True): - pass + if isinstance(empty, bool) and not isinstance(empty, str): + parameter.empty = empty + elif empty in empty_remap: + utils.deprecated_warn(warn_msg, version="5.0.0") + parameter.empty = empty_remap[empty] # type: ignore[index] else: - msg = "The value of 'empty' should be True or False." - raise ValueError(msg) + raise ValueError(warn_msg) - if "init" in kwds: - warnings.warn( - """Use 'value' instead of 'init'.""", - utils.AltairDeprecationWarning, - stacklevel=1, - ) + if _init := kwds.pop("init", None): + utils.deprecated_warn("Use `value` instead of `init`.", version="5.0.0") + # If both 'value' and 'init' are set, we ignore 'init'. if value is Undefined: - kwds["value"] = kwds.pop("init") - else: - # If both 'value' and 'init' are set, we ignore 'init'. - kwds.pop("init") + kwds["value"] = _init # ignore[arg-type] comment is needed because we can also pass _expr_core.Expression if "select" not in kwds: @@ -518,11 +501,10 @@ def _selection( select = core.PointSelectionConfig(type=type, **kwds) elif type in {"single", "multi"}: select = core.PointSelectionConfig(type="point", **kwds) - warnings.warn( - """The types 'single' and 'multi' are now - combined and should be specified using "selection_point()".""", - utils.AltairDeprecationWarning, - stacklevel=1, + utils.deprecated_warn( + "The types `single` and `multi` are now combined.", + version="5.0.0", + alternative="selection_point()", ) else: msg = """'type' must be 'point' or 'interval'""" @@ -1260,12 +1242,10 @@ def save( additional kwargs passed to spec_to_mimebundle. """ if webdriver is not None: - warnings.warn( - "The webdriver argument is deprecated as it's not relevant for" - + " the new vl-convert engine which replaced altair_saver." - + " The argument will be removed in a future release.", - AltairDeprecationWarning, - stacklevel=1, + utils.deprecated_warn( + "The webdriver argument is not relevant for the new vl-convert engine which replaced altair_saver. " + "The argument will be removed in a future release.", + version="5.0.0", ) from ...utils.save import save From def3066b41f6cc2cc201c569dff8d3eac4a02b9c Mon Sep 17 00:00:00 2001 From: dangotbanned <125183946+dangotbanned@users.noreply.github.com> Date: Mon, 15 Jul 2024 21:19:35 +0100 Subject: [PATCH 8/8] fix: add explicit top-level import `AltairDeprecationWarning` Resolves https://github.com/vega/altair/pull/3455#discussion_r1678237304 Note: The other imports changes are simply to align with my other PRs in review --- altair/__init__.py | 7 ++++--- altair/vegalite/v5/api.py | 1 - 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/altair/__init__.py b/altair/__init__.py index bd81f0209..ad77b5633 100644 --- a/altair/__init__.py +++ b/altair/__init__.py @@ -617,11 +617,12 @@ def __dir__(): return __all__ -from .vegalite import * -from .jupyter import JupyterChart +from altair.vegalite import * +from altair.jupyter import JupyterChart +from altair.utils import AltairDeprecationWarning def load_ipython_extension(ipython): - from ._magics import vegalite + from altair._magics import vegalite ipython.register_magic_function(vegalite, "cell") diff --git a/altair/vegalite/v5/api.py b/altair/vegalite/v5/api.py index 939f6a356..e9d420828 100644 --- a/altair/vegalite/v5/api.py +++ b/altair/vegalite/v5/api.py @@ -24,7 +24,6 @@ compile_with_vegafusion as _compile_with_vegafusion, ) from altair.utils.data import DataType, is_data_type as _is_data_type -from altair.utils.deprecation import AltairDeprecationWarning # noqa: F401 from altair.utils.core import ( to_eager_narwhals_dataframe as _to_eager_narwhals_dataframe, )