Skip to content

gh-95065: Add Argument Clinic support for deprecating positional use of parameters #95151

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

Merged
merged 95 commits into from
Aug 7, 2023
Merged
Show file tree
Hide file tree
Changes from 43 commits
Commits
Show all changes
95 commits
Select commit Hold shift + click to select a range
9196162
PoC
erlend-aasland Jun 24, 2022
6a8c14f
Add tests
erlend-aasland Jul 22, 2022
457d38a
Improve warning wording
erlend-aasland Jul 22, 2022
3687748
Add NEWS
erlend-aasland Jul 22, 2022
9c81ae1
Pull in main
erlend-aasland Feb 21, 2023
a5734d6
Pull in main
erlend-aasland Apr 29, 2023
1e9fd55
Pull in main
erlend-aasland Jul 2, 2023
93e0745
Merge branch 'main' into ac-deprecate
erlend-aasland Jul 3, 2023
6bde63d
Add annotation
erlend-aasland Jul 3, 2023
bb619a5
Pull in main
erlend-aasland Jul 3, 2023
d85475e
Amend tests
erlend-aasland Jul 3, 2023
68f8ed9
Pull in main
erlend-aasland Jul 3, 2023
ca496b4
Amend test
erlend-aasland Jul 3, 2023
fe77d5f
Annotate
erlend-aasland Jul 3, 2023
de9e661
Pull in main
erlend-aasland Jul 12, 2023
3701b41
Fix merge
erlend-aasland Jul 14, 2023
a149a7b
Fix tests and repr
erlend-aasland Jul 14, 2023
e9ad211
Get ready for '* [from ...]' syntax
erlend-aasland Jul 14, 2023
0fcad58
Add 'whence' to deprecation warning message
erlend-aasland Jul 14, 2023
efb1fba
PoC
erlend-aasland Jul 14, 2023
3d8164f
Pull in main
erlend-aasland Jul 15, 2023
43cec6c
Improve preprocessor warning message, and fix it on windows
erlend-aasland Jul 15, 2023
075cf67
Nits
erlend-aasland Jul 15, 2023
13104e8
Assert format
erlend-aasland Jul 15, 2023
dc22c91
Pull in main
erlend-aasland Jul 15, 2023
7864dcb
Improve accuracy of variable name
erlend-aasland Jul 15, 2023
fbed52f
Add docs
erlend-aasland Jul 15, 2023
222c3bb
Pull in main
erlend-aasland Jul 15, 2023
2abe7a3
Adjust CPP barking
erlend-aasland Jul 15, 2023
4c0ec79
Fix CPP
erlend-aasland Jul 15, 2023
35fbe80
Fix CPP for error case also
erlend-aasland Jul 15, 2023
ccf8df8
Update clinic.test.c
erlend-aasland Jul 15, 2023
4db3c8d
Fix and test multiple deprecation directives error message
erlend-aasland Jul 15, 2023
43af475
Extend the docs
erlend-aasland Jul 15, 2023
5074495
Add note about compiler error
erlend-aasland Jul 15, 2023
ce5cd92
Fixup docs and fixup cpp error string
erlend-aasland Jul 15, 2023
c2cfc80
Pull in main
erlend-aasland Jul 19, 2023
4dc8241
Fix mypy complain
erlend-aasland Jul 19, 2023
b2c8f34
Pull in main
erlend-aasland Jul 20, 2023
5f181c7
Pull in main
erlend-aasland Jul 20, 2023
9eab8f7
Address review: annotate deprecated_positional as str | None
AlexWaygood Jul 20, 2023
25aa23c
Address review: add return annotation to parse_deprecated_positional()
erlend-aasland Jul 20, 2023
6624e8f
Merge branch 'main' into ac-deprecate
erlend-aasland Jul 20, 2023
46924a7
Pull in main
erlend-aasland Jul 22, 2023
2e72b66
WIP
erlend-aasland Jul 22, 2023
cbbcf1a
Pull in main
erlend-aasland Aug 2, 2023
f82be40
Pull in main
erlend-aasland Aug 4, 2023
a76d63c
Move prototype to class level
erlend-aasland Aug 4, 2023
e5bbd7b
Address review: start warning from alpha stage
erlend-aasland Aug 4, 2023
c981efa
Add more test cases
erlend-aasland Aug 4, 2023
821124e
Add TODO comment
erlend-aasland Aug 4, 2023
aa1b85d
Test case 2 already covers this test case
erlend-aasland Aug 5, 2023
44af850
Handle more cases
erlend-aasland Aug 5, 2023
01f17f9
Add one more test case as requested by Nikita
erlend-aasland Aug 5, 2023
e6860bc
Fix mypy whining
erlend-aasland Aug 5, 2023
71a8750
Address Alex's review: solve mypy complaint more elegantly
erlend-aasland Aug 5, 2023
e6886f0
Address some of Serhiy's remarks:
erlend-aasland Aug 5, 2023
480a29f
Address review: use stack level 1
erlend-aasland Aug 5, 2023
a3cf43a
Merge branch 'main' into ac-deprecate
erlend-aasland Aug 5, 2023
5151e62
Pull in main
erlend-aasland Aug 5, 2023
b98aab3
Address Serhiy's second round of remarks:
erlend-aasland Aug 5, 2023
eeb8187
Fix merge: revert accidental removal of a clinic.test.c test method
erlend-aasland Aug 5, 2023
6c6dd10
Address Alex and Serhiy's doc remarks
erlend-aasland Aug 5, 2023
ddd319b
Pull in main
erlend-aasland Aug 6, 2023
47a8979
Further improvements:
erlend-aasland Aug 6, 2023
02eba96
Make mypy happy (quickfix)
erlend-aasland Aug 6, 2023
fa77489
Revert sqlite3 test
erlend-aasland Aug 6, 2023
87a7f20
Use try...except ValueError trick for neater code
erlend-aasland Aug 6, 2023
f36bc05
Move VersionTuple to above Parameter class
erlend-aasland Aug 6, 2023
5e65020
Add more tests for illegal formats
erlend-aasland Aug 6, 2023
f1f311a
Don't link to keyword-only param glossary section
erlend-aasland Aug 6, 2023
83de0b8
No need for iter
erlend-aasland Aug 6, 2023
b10dc9c
For now, only use the basename of the source file
erlend-aasland Aug 6, 2023
c4ba025
Apply suggestions from code review
erlend-aasland Aug 6, 2023
c875d60
Fix mypy error
erlend-aasland Aug 6, 2023
cdd927c
Fix deprecation condition
erlend-aasland Aug 6, 2023
f0847bc
Further improve warning messages
erlend-aasland Aug 6, 2023
ce8db9a
Simplify condition
erlend-aasland Aug 6, 2023
0dff20c
Merge remote-tracking branch 'origin/ac-deprecate' into ac-deprecate
erlend-aasland Aug 6, 2023
b45d845
Refactor: extract pprinter
erlend-aasland Aug 6, 2023
d2330c6
Reintroduce condition and fixup deprecation message
erlend-aasland Aug 6, 2023
622b556
Remove unused variable, and reorder some lines for readability
erlend-aasland Aug 6, 2023
e71fbe1
Simplify pprint_words
erlend-aasland Aug 6, 2023
c4141f0
Fix function name formatting
erlend-aasland Aug 6, 2023
87e50c7
Use fully qualified function/meth name in fail() messages
erlend-aasland Aug 6, 2023
6e9e7b4
Add more test cases and normalise the their naming
erlend-aasland Aug 6, 2023
bde0f90
Improve deprecation messages
erlend-aasland Aug 6, 2023
8e9ea05
Pull in main
erlend-aasland Aug 7, 2023
8324510
ValueError will catch if '.' not in thenceforth for us
erlend-aasland Aug 7, 2023
2e4d64f
Improve NEWS entry
erlend-aasland Aug 7, 2023
52e7078
Update Misc/NEWS.d/next/Tools-Demos/2022-07-23-00-33-28.gh-issue-9506…
erlend-aasland Aug 7, 2023
8a39ffb
Fix grammar in depr sentence
erlend-aasland Aug 7, 2023
dc8f10d
Merge remote-tracking branch 'origin/ac-deprecate' into ac-deprecate
erlend-aasland Aug 7, 2023
d2130ab
Fix fixup code and add test
erlend-aasland Aug 7, 2023
b21bc32
Revert "Fix fixup code and add test"
erlend-aasland Aug 7, 2023
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
79 changes: 79 additions & 0 deletions Doc/howto/clinic.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1824,3 +1824,82 @@ blocks embedded in Python files look slightly different. They look like this:
#[python start generated code]*/
def foo(): pass
#/*[python checksum:...]*/


How to deprecate positional use of optional parameters
------------------------------------------------------

Argument Clinic provides syntax that makes it possible to generate code that
deprecates positional use of optional parameters.
For example, say we've got a module level function :py:func:`!foo.myfunc` that
takes three :class:`int` parameters:
optional parameters *a* and *b*, and keyword-only parameter *c*::

/*[clinic input]
module foo
myfunc
a: int
b: int
*
c: int
[clinic start generated output]*/

We now want to make the *b* parameter keyword-only from Python 3.14 and onward,
and since :py:func:`!myfunc` is a public API,
we must follow :pep:`387` and issue a deprecation warning.
We can do that using the ``* [from ...]``` syntax,
by adding the line ``* [from 3.14]`` right above the *b* parameter::

/*[clinic input]
module foo
myfunc
a: int
* [from 3.14]
b: int
*
c: int
[clinic start generated output]*/

Next, regenerate Argument Clinic code (``make clinic``),
and add unit tests for the new behaviour.

The generated code will now emit a :exc:`DeprecationWarning`
when an the optional parameter *b* is used as a positional paremeter.
C preprocessor directives are also generated for emitting
compiler warnings if the ``* [from ...]`` line has not been removed
from the Argument Clinic input when the deprecation period is over,
which means when the beta phase of the specified Python version kicks in.

Let's return to our example and assume Python 3.14 development has entered the
beta phase, and we forgot all about updating the Argument Clinic code for
:py:func:`!myfunc`.
Luckily for us, compiler warnings are now generated:

.. code-block:: none

In file included from Modules/foomodule.c:139:
Modules/clinic/foomodule.c.h:83:8: warning: Update 'b' in 'myfunc' in 'foomodule.c' to be keyword-only. [-W#warnings]
# warning "Update 'b' in 'myfunc' in 'foomodule.c' to be keyword-only."
^

We now close the deprecation phase by making *b* keyword-only;
replace the ``* [from ...]``` line above *b*
with the ``*`` from the line above *c*::

/*[clinic input]
module foo
myfunc
a: int
*
b: int
c: int
[clinic start generated output]*/

Finally, run ``make clinic`` to regenerate the Argument Clinic code,
and update your unit tests to reflect the new behaviour.

.. note::

If you forget to update your clinic code during the target beta phase, the
copmiler warning will turn into a compiler error when the release candidate
phase kicks in.
116 changes: 116 additions & 0 deletions Lib/test/clinic.test.c
Original file line number Diff line number Diff line change
Expand Up @@ -4948,3 +4948,119 @@ Test_meth_coexist(TestObj *self, PyObject *Py_UNUSED(ignored))
static PyObject *
Test_meth_coexist_impl(TestObj *self)
/*[clinic end generated code: output=808a293d0cd27439 input=2a1d75b5e6fec6dd]*/


/*[clinic input]
test_deprecate_positional_use
pos: object
* [from 3.14]
optarg: int = 5
[clinic start generated code]*/

PyDoc_STRVAR(test_deprecate_positional_use__doc__,
"test_deprecate_positional_use($module, /, pos, optarg=5)\n"
"--\n"
"\n");

#define TEST_DEPRECATE_POSITIONAL_USE_METHODDEF \
{"test_deprecate_positional_use", _PyCFunction_CAST(test_deprecate_positional_use), METH_FASTCALL|METH_KEYWORDS, test_deprecate_positional_use__doc__},

static PyObject *
test_deprecate_positional_use_impl(PyObject *module, PyObject *pos,
int optarg);

static PyObject *
test_deprecate_positional_use(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames)
{
PyObject *return_value = NULL;
#if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE)

#define NUM_KEYWORDS 2
static struct {
PyGC_Head _this_is_not_used;
PyObject_VAR_HEAD
PyObject *ob_item[NUM_KEYWORDS];
} _kwtuple = {
.ob_base = PyVarObject_HEAD_INIT(&PyTuple_Type, NUM_KEYWORDS)
.ob_item = { &_Py_ID(pos), &_Py_ID(optarg), },
};
#undef NUM_KEYWORDS
#define KWTUPLE (&_kwtuple.ob_base.ob_base)

#else // !Py_BUILD_CORE
# define KWTUPLE NULL
#endif // !Py_BUILD_CORE

static const char * const _keywords[] = {"pos", "optarg", NULL};
static _PyArg_Parser _parser = {
.keywords = _keywords,
.fname = "test_deprecate_positional_use",
.kwtuple = KWTUPLE,
};
#undef KWTUPLE
PyObject *argsbuf[2];
Py_ssize_t noptargs = nargs + (kwnames ? PyTuple_GET_SIZE(kwnames) : 0) - 1;
PyObject *pos;
int optarg = 5;

args = _PyArg_UnpackKeywords(args, nargs, NULL, kwnames, &_parser, 1, 2, 0, argsbuf);
if (!args) {
goto exit;
}
pos = args[0];
if (!noptargs) {
goto skip_optional_pos;
}
#if PY_MAJOR_VERSION == 3 && \
PY_MINOR_VERSION == 14 && \
PY_RELEASE_LEVEL == PY_RELEASE_LEVEL_BETA
# ifdef _MSC_VER
# pragma message ("Update 'optarg' in 'test_deprecate_positional_use' in 'clinic.test.c' to be keyword-only.")
# else
# warning "Update 'optarg' in 'test_deprecate_positional_use' in 'clinic.test.c' to be keyword-only."
# endif
#elif PY_MAJOR_VERSION > 3 || \
(PY_MAJOR_VERSION == 3 && PY_MINOR_VERSION > 14) || \
(PY_MAJOR_VERSION == 3 && \
PY_MINOR_VERSION == 14 && \
PY_RELEASE_LEVEL == PY_RELEASE_LEVEL_GAMMA)
# error "Update 'optarg' in 'test_deprecate_positional_use' in 'clinic.test.c' to be keyword-only."
#endif
if (nargs == 2) {
if (PyErr_WarnEx(PyExc_DeprecationWarning,
"Using 'optarg' as a positional argument is deprecated. "
"It will become a keyword-only argument in Python 3.14.", 2))
{
goto exit;
}
}
optarg = _PyLong_AsInt(args[1]);
if (optarg == -1 && PyErr_Occurred()) {
goto exit;
}
skip_optional_pos:
return_value = test_deprecate_positional_use_impl(module, pos, optarg);

exit:
return return_value;
}

static PyObject *
test_deprecate_positional_use_impl(PyObject *module, PyObject *pos,
int optarg)
/*[clinic end generated code: output=7a11cf25f54b6385 input=f77517a893443884]*/

PyDoc_STRVAR(test_deprecate_positional_use__doc__,
"test_deprecate_positional_use($module, /, pos, optarg=5)\n"
"--\n"
"\n");

#define TEST_DEPRECATE_POSITIONAL_USE_METHODDEF \
{"test_deprecate_positional_use", _PyCFunction_CAST(test_deprecate_positional_use), METH_FASTCALL|METH_KEYWORDS, test_deprecate_positional_use__doc__},

static PyObject *
test_deprecate_positional_use_impl(PyObject *module, PyObject *pos,
int optarg);

static PyObject *
test_deprecate_positional_use(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames)
66 changes: 66 additions & 0 deletions Lib/test/test_clinic.py
Original file line number Diff line number Diff line change
Expand Up @@ -948,6 +948,72 @@ def test_parameters_required_after_star(self):
out = self.parse_function_should_fail(block)
self.assertIn(msg, out)

def test_depr_star_invalid_format_1(self):
out = self.parse_function_should_fail("""
module foo
foo.bar
this: int
* [from 3]
Docstring.
""")
expected = (
"Error on line 0:\n"
"Function 'bar': '* [from ...]' format expected to be "
"'<major.minor>', where 'major' and 'minor' are digits.\n"
)
self.assertEqual(out, expected)

def test_depr_star_invalid_format_2(self):
out = self.parse_function_should_fail("""
module foo
foo.bar
this: int
* [from a.b]
Docstring.
""")
expected = (
"Error on line 0:\n"
"Function 'bar', '* [from <major.minor>]': "
"'major' and 'minor' must be digits, not 'a' and 'b'\n"
)
self.assertEqual(out, expected)

def test_parameters_required_after_depr_star(self):
out = self.parse_function_should_fail("""
module foo
foo.bar
this: int
* [from 3.14]
Docstring.
""")
msg = "Function bar specifies '* [from ...]' without any parameters afterwards."
self.assertIn(msg, out)

def test_depr_star_must_come_before_star(self):
out = self.parse_function_should_fail("""
module foo
foo.bar
this: int
*
* [from 3.14]
Docstring.
""")
msg = "Function 'bar': '* [from ...]' must come before '*'"
self.assertIn(msg, out)

def test_duplicate_depr_star(self):
out = self.parse_function_should_fail("""
module foo
foo.bar
a: int
* [from 3.14]
* [from 3.14]
b: int
Docstring.
""")
msg = "Function 'bar' uses '[from ...]' more than once."
self.assertIn(msg, out)

def test_single_slash(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we are missing the deprecation warning check here. Calling test_deprecate_positional_use with deprecated positional must produce a runtime deprecation warning. That's its main feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems we could reuse some of the pegen test support code (see Tools/peg_generator/pegen, compile_c_extension, etc.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two scenarios we want to test:

  • deprecation warning emitted in Python (we can do this now, using some tricks in _testclinic.c)
  • C preprocessor warnings and error emitted at compile time (this needs a new framework, similar to what pegen does)

I suggest we start with the former. The latter would be nice to have, but it requires more invasive changes, so I suggest we do that in separate PRs post this PR.

out = self.parse_function_should_fail("""
module foo
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Add Argument Clinic support for deprecating positional use of optional
parameters. Patch by Erlend E. Aasland.
Loading