-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Add @deprecate_func
and @deprecate_arg
decorators
#9676
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
Changes from 1 commit
e44dae4
92669a3
c9416d3
9d8db91
7b7c592
fd577ea
7e7089a
be105a1
00a1af1
7d5c4b6
6e69253
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,7 +14,173 @@ | |
|
||
import functools | ||
import warnings | ||
from typing import Any, Callable, Dict, Optional, Type | ||
from typing import Any, Callable, Dict, Optional, Type, Tuple, Union | ||
|
||
|
||
def deprecate_func( | ||
*, | ||
since: str, | ||
additional_msg: Optional[str] = None, | ||
pending: bool = False, | ||
project_name: str = "Qiskit Terra", | ||
Eric-Arellano marked this conversation as resolved.
Show resolved
Hide resolved
|
||
removal_timeline: str = "no earlier than 3 months after the release date", | ||
is_property: bool = False, | ||
): | ||
"""Decorator to indicate a function has been deprecated. | ||
|
||
It should be placed beneath other decorators like `@staticmethod` and property decorators. | ||
|
||
When deprecating a class, set this decorator on its `__init__` function. | ||
|
||
Args: | ||
since: The version the deprecation started at. If the deprecation is pending, set | ||
the version to when that started; but later, when switching from pending to | ||
deprecated, update `since` to the new version. | ||
Eric-Arellano marked this conversation as resolved.
Show resolved
Hide resolved
|
||
additional_msg: Put here any additional information, such as what to use instead. | ||
For example, "Instead, use the function `new_func` from the module `qiskit.my_module`, | ||
which is similar but uses GPU acceleration." | ||
pending: Set to `True` if the deprecation is still pending. | ||
Eric-Arellano marked this conversation as resolved.
Show resolved
Hide resolved
|
||
project_name: The name of the project, e.g. "Qiskit Nature". | ||
removal_timeline: How soon can this deprecation be removed? Expects a value | ||
like "no sooner than 6 months after the latest release" or "in release 9.99". | ||
is_property: If the deprecated function is a `@property`, set this to True so that the | ||
generated message correctly describes it as such. (This isn't necessary for | ||
property setters, as their docstring is ignored by Python.) | ||
|
||
Returns: | ||
Callable: The decorated callable. | ||
""" | ||
|
||
def decorator(func): | ||
qualname = func.__qualname__ # For methods, `qualname` includes the class name. | ||
mod_name = func.__module__ | ||
|
||
# Detect what function type this is. | ||
if is_property: | ||
# `inspect.isdatadescriptor()` doesn't work because you must apply our decorator | ||
# before `@property`, so it looks like the function is a normal method. | ||
deprecated_entity = f"The property ``{mod_name}.{qualname}``" | ||
# To determine if's a method, we use the heuristic of looking for a `.` in the qualname. | ||
# This is because top-level functions will only have the function name. This is not | ||
# perfect, e.g. it incorrectly classifies nested/inner functions, but we don't expect | ||
# those to be deprecated. | ||
# | ||
# We can't use `inspect.ismethod()` because that only works when calling it on an instance | ||
# of the class, rather than the class type itself, i.e. `ismethod(C().foo)` vs | ||
# `ismethod(C.foo)`. | ||
elif "." in qualname: | ||
if func.__name__ == "__init__": | ||
cls_name = qualname[: -len(".__init__")] | ||
deprecated_entity = f"The class ``{mod_name}.{cls_name}``" | ||
else: | ||
deprecated_entity = f"The method ``{mod_name}.{qualname}()``" | ||
else: | ||
deprecated_entity = f"The function ``{mod_name}.{qualname}()``" | ||
|
||
msg, category = _write_deprecation_msg( | ||
deprecated_entity=deprecated_entity, | ||
project_name=project_name, | ||
since=since, | ||
pending=pending, | ||
additional_msg=additional_msg, | ||
removal_timeline=removal_timeline, | ||
) | ||
|
||
@functools.wraps(func) | ||
def wrapper(*args, **kwargs): | ||
warnings.warn(msg, category=category, stacklevel=2) | ||
return func(*args, **kwargs) | ||
|
||
add_deprecation_to_docstring(wrapper, msg, since=since, pending=pending) | ||
return wrapper | ||
|
||
return decorator | ||
|
||
|
||
def deprecate_arg( | ||
name: str, | ||
*, | ||
since: str, | ||
additional_msg: Optional[str] = None, | ||
deprecation_description: Optional[str] = None, | ||
pending: bool = False, | ||
project_name: str = "Qiskit Terra", | ||
new_alias: Optional[str] = None, | ||
predicate: Optional[Callable[[Any], bool]] = None, | ||
removal_timeline: str = "no earlier than 3 months after the release date", | ||
): | ||
"""Decorator to indicate an argument has been deprecated in some way. | ||
|
||
This decorator may be used multiple times on the same function, once per deprecated argument. | ||
It should be placed beneath other decorators like `@staticmethod` and property decorators. | ||
Eric-Arellano marked this conversation as resolved.
Show resolved
Hide resolved
Eric-Arellano marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Args: | ||
name: The name of the deprecated argument. | ||
since: The version the deprecation started at. If the deprecation is pending, set | ||
the version to when that started; but later, when switching from pending to | ||
deprecated, update `since` to the new version. | ||
deprecation_description: What is being deprecated? E.g. "Setting my_func()'s `my_arg` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't love this name. I considered also There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that |
||
argument to `None`." If not set, will default to "{func_name}'s argument `{name}`". | ||
additional_msg: Put here any additional information, such as what to use instead | ||
(if new_alias is not set). For example, "Instead, use the argument `new_arg`, | ||
which is similar but does not impact the circuit's setup." | ||
pending: Set to `True` if the deprecation is still pending. | ||
project_name: The name of the project, e.g. "Qiskit Nature". | ||
new_alias: If the arg has simply been renamed, set this to the new name. The decorator will | ||
dynamically update the `kwargs` so that when the user sets the old arg, it will be | ||
passed in as the `new_alias` arg. | ||
predicate: Only log the runtime warning if the predicate returns True. This is useful to | ||
deprecate certain values or types for an argument, e.g. | ||
`lambda my_arg: isinstance(my_arg, dict)`. Regardless of if a predicate is set, the | ||
runtime warning will only log when the user specifies the argument. | ||
removal_timeline: How soon can this deprecation be removed? Expects a value | ||
like "no sooner than 6 months after the latest release" or "in release 9.99". | ||
|
||
Returns: | ||
Callable: The decorated callable. | ||
""" | ||
|
||
def decorator(func): | ||
# For methods, `__qualname__` includes the class name. | ||
func_name = f"{func.__module__}.{func.__qualname__}()" | ||
deprecated_entity = deprecation_description or f"``{func_name}``'s argument ``{name}``" | ||
|
||
if new_alias: | ||
alias_msg = f"Instead, use the argument ``{new_alias}``, which behaves identically." | ||
if additional_msg: | ||
final_additional_msg = f"{alias_msg}. {additional_msg}" | ||
else: | ||
final_additional_msg = alias_msg | ||
else: | ||
final_additional_msg = additional_msg | ||
|
||
msg, category = _write_deprecation_msg( | ||
deprecated_entity=deprecated_entity, | ||
project_name=project_name, | ||
since=since, | ||
pending=pending, | ||
additional_msg=final_additional_msg, | ||
removal_timeline=removal_timeline, | ||
) | ||
|
||
@functools.wraps(func) | ||
def wrapper(*args, **kwargs): | ||
if kwargs: | ||
_maybe_warn_and_rename_kwarg( | ||
func_name, | ||
kwargs, | ||
old_arg=name, | ||
new_alias=new_alias, | ||
warning_msg=msg, | ||
category=category, | ||
predicate=predicate, | ||
) | ||
return func(*args, **kwargs) | ||
|
||
add_deprecation_to_docstring(wrapper, msg, since=since, pending=pending) | ||
return wrapper | ||
|
||
return decorator | ||
|
||
|
||
def deprecate_arguments( | ||
|
@@ -23,7 +189,7 @@ def deprecate_arguments( | |
*, | ||
since: Optional[str] = None, | ||
): | ||
"""Decorator to automatically alias deprecated argument names and warn upon use. | ||
"""Deprecated. Instead, use `@deprecate_arg`. | ||
Comment on lines
-26
to
+192
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure how we should deprecate this -- should we use |
||
|
||
Args: | ||
kwarg_map: A dictionary of the old argument name to the new name. | ||
|
@@ -51,7 +217,16 @@ def decorator(func): | |
@functools.wraps(func) | ||
def wrapper(*args, **kwargs): | ||
if kwargs: | ||
_rename_kwargs(func_name, kwargs, old_kwarg_to_msg, kwarg_map, category) | ||
for old, new in kwarg_map.items(): | ||
_maybe_warn_and_rename_kwarg( | ||
func_name, | ||
kwargs, | ||
old_arg=old, | ||
new_alias=new, | ||
warning_msg=old_kwarg_to_msg[old], | ||
category=category, | ||
predicate=None, | ||
) | ||
return func(*args, **kwargs) | ||
|
||
for msg in old_kwarg_to_msg.values(): | ||
|
@@ -70,7 +245,7 @@ def deprecate_function( | |
*, | ||
since: Optional[str] = None, | ||
): | ||
"""Emit a warning prior to calling decorated function. | ||
"""Deprecated. Instead, use `@deprecate_func`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto that I'm not sure if we should use |
||
|
||
Args: | ||
msg: Warning message to emit. | ||
|
@@ -99,21 +274,52 @@ def wrapper(*args, **kwargs): | |
return decorator | ||
|
||
|
||
def _rename_kwargs( | ||
def _maybe_warn_and_rename_kwarg( | ||
func_name: str, | ||
kwargs: Dict[str, Any], | ||
old_kwarg_to_msg: Dict[str, str], | ||
kwarg_map: Dict[str, Optional[str]], | ||
category: Type[Warning] = DeprecationWarning, | ||
*, | ||
old_arg: str, | ||
new_alias: Optional[str], | ||
warning_msg: str, | ||
category: Type[Warning], | ||
predicate: Optional[Callable[[Any], bool]], | ||
) -> None: | ||
for old_arg, new_arg in kwarg_map.items(): | ||
if old_arg not in kwargs: | ||
continue | ||
if new_arg in kwargs: | ||
raise TypeError(f"{func_name} received both {new_arg} and {old_arg} (deprecated).") | ||
warnings.warn(old_kwarg_to_msg[old_arg], category=category, stacklevel=3) | ||
if new_arg is not None: | ||
kwargs[new_arg] = kwargs.pop(old_arg) | ||
if old_arg not in kwargs: | ||
return | ||
if new_alias and new_alias in kwargs: | ||
raise TypeError(f"{func_name} received both {new_alias} and {old_arg} (deprecated).") | ||
if predicate and not predicate(kwargs[old_arg]): | ||
return | ||
warnings.warn(warning_msg, category=category, stacklevel=3) | ||
if new_alias is not None: | ||
kwargs[new_alias] = kwargs.pop(old_arg) | ||
|
||
|
||
def _write_deprecation_msg( | ||
*, | ||
deprecated_entity: str, | ||
project_name: str, | ||
since: str, | ||
pending: bool, | ||
additional_msg: str, | ||
removal_timeline: str, | ||
) -> Tuple[str, Union[Type[DeprecationWarning], Type[PendingDeprecationWarning]]]: | ||
if pending: | ||
category = PendingDeprecationWarning | ||
deprecation_status = "pending deprecation" | ||
removal_desc = f"marked deprecated in a future release, and then removed {removal_timeline}" | ||
else: | ||
category = DeprecationWarning | ||
deprecation_status = "deprecated" | ||
removal_desc = f"removed {removal_timeline}" | ||
|
||
msg = ( | ||
f"{deprecated_entity} is {deprecation_status} as of {project_name} {since}. " | ||
f"It will be {removal_desc}." | ||
) | ||
if additional_msg: | ||
msg += f" {additional_msg}" | ||
return msg, category | ||
|
||
|
||
# We insert deprecations in-between the description and Napoleon's meta sections. The below is from | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm personally not a fan of adding these helpers to
utils/__init__.py
. I think it's better to be explicit and importqiskit.utils.deprecation
. For example, that helps to avoid issues with circular imports.But I also want these helpers to show up in the docs, and this seems to be the convention we use?