-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
gh-52597: Add position-only markers for os functions #94735
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
Conversation
@JelleZijlstra Could you take a peek at this one since you'd looked over the docs for |
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.
Thanks! I used this also for fixing a few missing pos-only markers in typeshed: python/typeshed#8283, python/typeshed#8284.
@@ -4538,7 +4538,7 @@ The following functions take a process status code as returned by | |||
:func:`system`, :func:`wait`, or :func:`waitpid` as a parameter. They may be | |||
used to determine the disposition of a process. | |||
|
|||
.. function:: WCOREDUMP(status) | |||
.. function:: WCOREDUMP(status, /) |
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.
Strangely enough, this one is indeed pos-only, but all the other similar ones below it aren't.
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.
Looking at
cpython/Modules/clinic/posixmodule.c.h
Lines 4282 to 4318 in 1009bf1
"WCOREDUMP($module, status, /)\n" | |
"--\n" | |
"\n" | |
"Return True if the process returning status was dumped to a core file."); | |
#define OS_WCOREDUMP_METHODDEF \ | |
{"WCOREDUMP", (PyCFunction)os_WCOREDUMP, METH_VARARGS, os_WCOREDUMP__doc__}, | |
static int | |
os_WCOREDUMP_impl(PyModuleDef *module, int status); | |
static PyObject * | |
os_WCOREDUMP(PyModuleDef *module, PyObject *args) | |
{ | |
PyObject *return_value = NULL; | |
int status; | |
int _return_value; | |
if (!PyArg_ParseTuple(args, | |
"i:WCOREDUMP", | |
&status)) | |
goto exit; | |
_return_value = os_WCOREDUMP_impl(module, status); | |
if ((_return_value == -1) && PyErr_Occurred()) | |
goto exit; | |
return_value = PyBool_FromLong((long)_return_value); | |
exit: | |
return return_value; | |
} | |
#endif /* defined(HAVE_SYS_WAIT_H) && defined(WCOREDUMP) */ | |
#if defined(HAVE_SYS_WAIT_H) && defined(WIFCONTINUED) | |
PyDoc_STRVAR(os_WIFCONTINUED__doc__, | |
"WIFCONTINUED($module, /, status)\n" |
Separately, could you check whether you're able to run os.plock(op=None)
to verify the keyword can't be used? I gave all the functions a quick run to double-check but like chflags
, I'm not able to run plock
for some reason.
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.
Actually, seeing that the clinic def
cpython/Modules/clinic/posixmodule.c.h
Lines 3688 to 3692 in f0bf795
PyDoc_STRVAR(os_plock__doc__, | |
"plock($module, op, /)\n" | |
"--\n" | |
"\n" | |
"Lock program segments into memory.\");"); |
has the position-only marker, then regardless of whether it can be confirmed, adding the plock
change would arguably be strictly better than the current docs. In that case, tracking this particular problem can go into some other issue instead, and I'd reckon that the change can go ahead anyways, if there aren't any other problems with the PR.
Thanks @slateny for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11. |
Sorry @slateny and @JelleZijlstra, I had trouble checking out the |
Sorry, @slateny and @JelleZijlstra, I could not cleanly backport this to |
Thanks @slateny for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11. |
GH-97643 is a backport of this pull request to the 3.11 branch. |
…94735) (cherry picked from commit c759944) Co-authored-by: Stanley <[email protected]>
(cherry picked from commit c759944) Co-authored-by: Stanley <[email protected]>
(cherry picked from commit c759944) Co-authored-by: Stanley <[email protected]>
#52597
chflags
skipped as I couldn't test it due to someAttributeError
, but per clinicfollow_symlinks
shouldn't be keyword-only.For verification, first to get all functions changed:
To show the params in the docs:
To show the params in clinic:
And combined:
The last command shows any differences in positional/keyword markers or param names. On visual inspection, all diffs just seem to be differences in param names so the markers in the doc are correct.