-
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
@deprecate_arguments
and @deprecate_function
add deprecation to docstring
#9790
Changes from 1 commit
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 |
---|---|---|
|
@@ -15,7 +15,89 @@ | |
from textwrap import dedent | ||
|
||
from qiskit.test import QiskitTestCase | ||
from qiskit.utils.deprecation import add_deprecation_to_docstring | ||
from qiskit.utils.deprecation import ( | ||
add_deprecation_to_docstring, | ||
deprecate_function, | ||
deprecate_arguments, | ||
) | ||
|
||
|
||
class TestDeprecationDecorators(QiskitTestCase): | ||
"""Test that the decorators in ``utils.deprecation`` correctly log warnings and get added to | ||
docstring.""" | ||
|
||
def test_deprecate_arguments_message(self) -> None: | ||
"""Test that `@deprecate_arguments` adds the correct message to the docstring.""" | ||
|
||
@deprecate_arguments( | ||
{"old_arg1": "new_arg1", "old_arg2": None}, | ||
category=PendingDeprecationWarning, | ||
since="9.99", | ||
) | ||
def my_func() -> None: | ||
pass | ||
|
||
self.assertEqual( | ||
my_func.__doc__, | ||
dedent( | ||
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. not sure about removing idents, as they are kinda important in this context. Do you think that a literal comparison is too strong? 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. This still considers indentation. Note how there are two spaces underneath the line Also, for context, these tests are not focused on ensuring that
|
||
f"""\ | ||
|
||
.. deprecated:: 9.99_pending | ||
{my_func.__qualname__} keyword argument old_arg1 is deprecated and replaced with \ | ||
new_arg1. | ||
|
||
.. deprecated:: 9.99_pending | ||
{my_func.__qualname__} keyword argument old_arg2 is deprecated and will in the \ | ||
future be removed. | ||
ElePT marked this conversation as resolved.
Show resolved
Hide resolved
|
||
""" | ||
), | ||
) | ||
|
||
def test_deprecate_function_docstring(self) -> None: | ||
"""Test that `@deprecate_function` adds the correct message to the docstring.""" | ||
|
||
@deprecate_function("Stop using my_func!", since="9.99") | ||
def my_func() -> None: | ||
pass | ||
|
||
self.assertEqual( | ||
my_func.__doc__, | ||
dedent( | ||
"""\ | ||
|
||
.. deprecated:: 9.99 | ||
Stop using my_func! | ||
""" | ||
), | ||
) | ||
|
||
def test_deprecate_arguments_runtime_warning(self) -> None: | ||
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. None of this functionality is new. But I wanted to make sure I didn't break anything. |
||
"""Test that `@deprecate_arguments` warns whenever the arguments are used. | ||
|
||
Also check that old arguments are passed in as their new alias. | ||
""" | ||
|
||
@deprecate_arguments({"arg1": None, "arg2": "new_arg2"}, since="9.99") | ||
def my_func(*, arg1: str = "a", new_arg2: str) -> None: | ||
del arg1 | ||
self.assertEqual(new_arg2, "z") | ||
|
||
my_func(new_arg2="z") # No warnings if no deprecated args used. | ||
with self.assertWarnsRegex(DeprecationWarning, "arg1"): | ||
my_func(arg1="a", new_arg2="z") | ||
with self.assertWarnsRegex(DeprecationWarning, "arg2"): | ||
# `arg2` should be converted into `new_arg2`. | ||
my_func(arg2="z") # pylint: disable=missing-kwoa | ||
|
||
def test_deprecate_function_runtime_warning(self) -> None: | ||
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 this functionality is not new |
||
"""Test that `@deprecate_function` warns whenever the function is used.""" | ||
|
||
@deprecate_function("Stop using my_func!", since="9.99") | ||
def my_func() -> None: | ||
pass | ||
|
||
with self.assertWarnsRegex(DeprecationWarning, "Stop using my_func!"): | ||
my_func() | ||
|
||
|
||
class AddDeprecationDocstringTest(QiskitTestCase): | ||
|
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.
All this diff does is convert from a multiline string to string concatenation. We can't handle
\n
inadd_deprecation_to_docstring()
(as a simplification)