-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[WAIT TO MERGE] Store deprecation metadata on functions for docs generation #9611
Changes from all commits
db5cdf8
718f562
4ab0875
ecf6d75
b879bce
2162cd8
01cadaa
bdf104b
2768c44
50f3a95
6a942cc
bfb90a6
59be9e7
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,11 +14,12 @@ | |
|
||
import functools | ||
import warnings | ||
from typing import Any, Dict, Optional, Type | ||
from dataclasses import dataclass | ||
from typing import Any, Callable, ClassVar, Dict, Optional, Type | ||
|
||
|
||
def deprecate_arguments( | ||
kwarg_map: Dict[str, str], | ||
kwarg_map: Dict[str, Optional[str]], | ||
category: Type[Warning] = DeprecationWarning, | ||
*, | ||
since: Optional[str] = None, | ||
|
@@ -37,15 +38,27 @@ def deprecate_arguments( | |
Callable: The decorated callable. | ||
""" | ||
|
||
del since # Will be used in a followup to add deprecations to our docs site. | ||
|
||
def decorator(func): | ||
func_name = func.__name__ | ||
old_kwarg_to_msg = {} | ||
for old_arg, new_arg in kwarg_map.items(): | ||
msg_suffix = ( | ||
"will in the future be removed." if new_arg is None else f"replaced with {new_arg}." | ||
) | ||
old_kwarg_to_msg[ | ||
old_arg | ||
] = f"{func_name} keyword argument {old_arg} is deprecated and {msg_suffix}" | ||
|
||
@functools.wraps(func) | ||
def wrapper(*args, **kwargs): | ||
if kwargs: | ||
_rename_kwargs(func.__name__, kwargs, kwarg_map, category) | ||
_rename_kwargs(func_name, kwargs, old_kwarg_to_msg, kwarg_map, category) | ||
return func(*args, **kwargs) | ||
|
||
for msg in old_kwarg_to_msg.values(): | ||
_DeprecationMetadataEntry( | ||
msg, since=since, pending=issubclass(category, PendingDeprecationWarning) | ||
).store_on_function(wrapper) | ||
return wrapper | ||
|
||
return decorator | ||
|
@@ -73,14 +86,15 @@ def deprecate_function( | |
Callable: The decorated, deprecated callable. | ||
""" | ||
|
||
del since # Will be used in a followup to add deprecations to our docs site. | ||
|
||
def decorator(func): | ||
@functools.wraps(func) | ||
def wrapper(*args, **kwargs): | ||
warnings.warn(msg, category=category, stacklevel=stacklevel) | ||
return func(*args, **kwargs) | ||
|
||
_DeprecationMetadataEntry( | ||
msg=msg, since=since, pending=issubclass(category, PendingDeprecationWarning) | ||
).store_on_function(wrapper) | ||
return wrapper | ||
|
||
return decorator | ||
|
@@ -89,27 +103,37 @@ def wrapper(*args, **kwargs): | |
def _rename_kwargs( | ||
func_name: str, | ||
kwargs: Dict[str, Any], | ||
kwarg_map: Dict[str, str], | ||
old_kwarg_to_msg: Dict[str, str], | ||
kwarg_map: Dict[str, Optional[str]], | ||
category: Type[Warning] = DeprecationWarning, | ||
) -> None: | ||
for old_arg, new_arg in kwarg_map.items(): | ||
if old_arg in kwargs: | ||
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 inverted the conditional to early return via |
||
if new_arg in kwargs: | ||
raise TypeError(f"{func_name} received both {new_arg} and {old_arg} (deprecated).") | ||
|
||
if new_arg is None: | ||
warnings.warn( | ||
f"{func_name} keyword argument {old_arg} is deprecated and " | ||
"will in future be removed.", | ||
category=category, | ||
stacklevel=3, | ||
) | ||
else: | ||
warnings.warn( | ||
f"{func_name} keyword argument {old_arg} is deprecated and " | ||
f"replaced with {new_arg}.", | ||
category=category, | ||
stacklevel=3, | ||
) | ||
|
||
kwargs[new_arg] = kwargs.pop(old_arg) | ||
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) | ||
|
||
|
||
@dataclass(frozen=True) | ||
class _DeprecationMetadataEntry: | ||
"""Used to store deprecation information on a function. | ||
|
||
This is used by the Qiskit meta repository to render deprecations in documentation. Warning: | ||
changes may accidentally break the meta repository; pay attention to backwards compatibility. | ||
""" | ||
|
||
msg: str | ||
since: Optional[str] | ||
pending: bool | ||
|
||
Eric-Arellano marked this conversation as resolved.
Show resolved
Hide resolved
|
||
dunder_name: ClassVar[str] = "__qiskit_deprecations__" | ||
|
||
def store_on_function(self, func: Callable) -> None: | ||
"""Add this metadata to the function's `__qiskit_deprecations__` attribute.""" | ||
if hasattr(func, self.dunder_name): | ||
getattr(func, self.dunder_name).append(self) | ||
else: | ||
setattr(func, self.dunder_name, [self]) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
# This code is part of Qiskit. | ||
# | ||
# (C) Copyright IBM 2023. | ||
# | ||
# This code is licensed under the Apache License, Version 2.0. You may | ||
# obtain a copy of this license in the LICENSE.txt file in the root directory | ||
# of this source tree or at http://www.apache.org/licenses/LICENSE-2.0. | ||
# | ||
# Any modifications or derivative works of this code must retain this | ||
# copyright notice, and modified files need to carry a notice indicating | ||
# that they have been altered from the originals. | ||
|
||
"""Tests for the functions in ``utils.deprecation``.""" | ||
|
||
from qiskit.test import QiskitTestCase | ||
from qiskit.utils.deprecation import ( | ||
_DeprecationMetadataEntry, | ||
deprecate_function, | ||
deprecate_arguments, | ||
) | ||
|
||
|
||
class TestDeprecations(QiskitTestCase): | ||
"""Test functions in ``utils.deprecation``.""" | ||
|
||
def test_deprecations_store_metadata(self) -> None: | ||
"""Test that our deprecation decorators store the metadata in __qiskit_deprecations__. | ||
|
||
This should support multiple deprecations on the same function. | ||
""" | ||
|
||
@deprecate_function("Stop using my_func!", since="9.99") | ||
@deprecate_arguments( | ||
{"old_arg": "new_arg"}, category=PendingDeprecationWarning, since="9.99" | ||
) | ||
def my_func(old_arg: int, new_arg: int) -> None: | ||
del old_arg | ||
del new_arg | ||
Comment on lines
+37
to
+38
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. Are these lines added here in order to pass linting? 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. Exactly. Pylint is overly pedantic |
||
|
||
self.assertEqual( | ||
getattr(my_func, _DeprecationMetadataEntry.dunder_name), | ||
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. Will this return the whole list of deprecation metadata associated with the deprecated function and deprecated arguments? 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. Yeah, the attribute |
||
[ | ||
_DeprecationMetadataEntry( | ||
"my_func keyword argument old_arg is deprecated and replaced with new_arg.", | ||
since="9.99", | ||
pending=True, | ||
), | ||
_DeprecationMetadataEntry("Stop using my_func!", since="9.99", pending=False), | ||
], | ||
) |
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.
This logic needs to live here, rather than the helper function
_rename_kwargs
, because we need to call_DeprecationMetadataEntry().store_on_function()
on thewrapper
rather than the innerfunc