gh-91118: Fix docstrings that do not honor --without-doc-strings by arhadthedev · Pull Request #31769 · python/cpython

arhadthedev · 2022-03-08T19:01:27Z

To support --without-doc-strings, all docstrings must be wrapped into PyDoc_STRVAR or PyDoc_STR (PEP 7). However, there are 18 occurrences in code and 10 in C API documentation that do not follow this rule. The documentation is important too because it should not teach people the wrong things.

To find the occurrences I searched for (?:^\s*.tp_doc = "|" \/\* tp_doc \*\/$) and^(?:static\s+)?const\s+char\s+[^=]+=\s*".

FixesGH-91118.

JelleZijlstra

There are a bunch of others, mostly in ctypes:

% git grep '".*tp_doc\s*'
Doc/c-api/typeobj.rst: "My objects", /* tp_doc */
Modules/_ctypes/_ctypes.c: "deletes a key from a dictionary", /* tp_doc */
Modules/_ctypes/_ctypes.c: "metatype for the CData Objects", /* tp_doc */
Modules/_ctypes/_ctypes.c: "metatype for the CData Objects", /* tp_doc */
Modules/_ctypes/_ctypes.c: "metatype for the Pointer Objects", /* tp_doc */
Modules/_ctypes/_ctypes.c: "metatype for the Array Objects", /* tp_doc */
Modules/_ctypes/_ctypes.c: "metatype for the PyCSimpleType Objects", /* tp_doc */
Modules/_ctypes/_ctypes.c: "metatype for C function pointers", /* tp_doc */
Modules/_ctypes/_ctypes.c: "XXX to be provided", /* tp_doc */
Modules/_ctypes/_ctypes.c: "Function Pointer", /* tp_doc */
Modules/_ctypes/_ctypes.c: "Structure base class", /* tp_doc */
Modules/_ctypes/_ctypes.c: "Union base class", /* tp_doc */
Modules/_ctypes/_ctypes.c: "XXX to be provided", /* tp_doc */
Modules/_ctypes/_ctypes.c: "XXX to be provided", /* tp_doc */
Modules/_ctypes/_ctypes.c: "XXX to be provided", /* tp_doc */
Modules/_ctypes/callbacks.c: "CThunkObject", /* tp_doc */
Modules/_ctypes/cfield.c: "Structure/Union member", /* tp_doc */
Modules/_testcapimodule.c: "Instantiating this exception starts infinite recursion.", /* tp_doc */

I'm a bit skeptical about how useful this is, though—does anyone actually use the configure option to strip docstrings?

JelleZijlstra · 2022-03-08T19:19:56Z

+:func:`ctypes.CopyComPointer`, :func:`ctypes.FormatError`,
+:func:`ctypes.FreeLibrary`, :func:`ctypes.LoadLibrary`, and
+:func:`ctypes.sizeof`, along with a bunch of methods in
+:class:`~_ctypes.PyCPointerType:, :class:`~_ctypes.PyCSimpleType:, and


Suggested change
:class:`~_ctypes.PyCPointerType:, :class:`~_ctypes.PyCSimpleType:, and
:class:`~_ctypes.PyCPointerType`, :class:`~_ctypes.PyCSimpleType`, and

JelleZijlstra · 2022-03-08T19:20:03Z

+:func:`ctypes.FreeLibrary`, :func:`ctypes.LoadLibrary`, and
+:func:`ctypes.sizeof`, along with a bunch of methods in
+:class:`~_ctypes.PyCPointerType:, :class:`~_ctypes.PyCSimpleType:, and
+:class:`~_ctypes.PyCStructType:.


Suggested change
:class:`~_ctypes.PyCStructType:.
:class:`~_ctypes.PyCStructType`.

I'm a bit skeptical about how useful this is, though—does anyone actually use the configure option to strip docstrings?
I think that if a flag exists, it should work fully. Also it's enabled by default so strings that should not be in the binary are no longer there.

…IVe9I.rst Co-authored-by: Éric <merwok@netwok.org>

miss-islington · 2022-04-18T03:40:52Z

Thanks @arhadthedev for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

miss-islington · 2022-04-18T03:40:53Z

Thanks @arhadthedev for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

miss-islington · 2022-04-18T03:40:58Z

Sorry, @arhadthedev and @JelleZijlstra, I could not cleanly backport this to 3.9 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker a573cb2fec664c645ab744658d7e941d72e1a398 3.9

miss-islington · 2022-04-18T03:41:00Z

Sorry @arhadthedev and @JelleZijlstra, I had trouble checking out the 3.10 backport branch.
Please backport using cherry_picker on command line.
cherry_picker a573cb2fec664c645ab744658d7e941d72e1a398 3.10

JelleZijlstra · 2022-04-18T03:42:00Z

@arhadthedev do you want to do the backports? I feel like the docs changes are most important to backport.

…-strings (pythonGH-31769) Co-authored-by: Éric <merwok@netwok.org> Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com> (cherry picked from commit a573cb2) Co-authored-by: Oleg Iarygin <oleg@arhadthedev.net>

bedevere-bot · 2022-04-18T12:33:54Z

GH-91662 is a backport of this pull request to the 3.10 branch.

python#31769) Co-authored-by: Éric <merwok@netwok.org> Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com> (cherry picked from commit a573cb2)

bedevere-bot · 2022-04-18T13:01:58Z

GH-91663 is a backport of this pull request to the 3.9 branch.

arhadthedev · 2022-04-18T13:17:22Z

That backport to 3.9 was targeted to main instead of 3.9.

The new, correct entry is GH-91664.

GH-31769) (#91664) Co-authored-by: Éric <merwok@netwok.org> Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com> (cherry picked from commit a573cb2)

…gs (GH-31769) (#91662) Co-authored-by: Éric <merwok@netwok.org> Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com> (cherry picked from commit a573cb2) Co-authored-by: Oleg Iarygin <oleg@arhadthedev.net>

…strings (pythonGH-31769) (python#91664) Co-authored-by: Éric <merwok@netwok.org> Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com> (cherry picked from commit a573cb2)

arhadthedev added 3 commits March 8, 2022 21:21

Fix inlined docstrings without PyDoc_STR
d6db6ca

Fix sample docstrings to show good PEP 7 practice
4a4f018

Fix non-inlined docstrings without PyDoc_STRVAR
1a87604

bedevere-bot added the awaiting review label Mar 8, 2022

the-knights-who-say-ni added the CLA signed label Mar 8, 2022

arhadthedev added 5 commits March 8, 2022 22:12

Add NEWS entries
745c887

Fix a typo
b04a234

Clarify a NEWS entry for Core and Builtins
a9c6225

Fix a typo 2
cf019f6

The module is called ctypes, not _ctypes
e5d5613

arhadthedev marked this pull request as ready for review March 8, 2022 19:52

JelleZijlstra reviewed Mar 9, 2022
View reviewed changes

Address a review
62da070

merwok reviewed Mar 12, 2022
View reviewed changes

Comment threadMisc/NEWS.d/next/Core and Builtins/2022-03-08-21-59-57.bpo-46962.UomDfz.rst Outdated

merwok reviewed Mar 12, 2022
View reviewed changes

Comment threadMisc/NEWS.d/next/Documentation/2022-03-08-22-10-38.bpo-46962.FIVe9I.rst Outdated

arhadthedevand others added 2 commits March 12, 2022 23:24

Update Misc/NEWS.d/next/Documentation/2022-03-08-22-10-38.bpo-46962.F…
87ebe0e
…IVe9I.rst Co-authored-by: Éric <merwok@netwok.org>

Address a review
a4a94be

arhadthedev changed the title ~~bpo-46962: Fix docstrings that do not honor --without-doc-strings~~gh-91118: Fix docstrings that do not honor --without-doc-stringsApr 10, 2022

arhadthedev closed this Apr 17, 2022

arhadthedev reopened this Apr 17, 2022

JelleZijlstra self-requested a review April 17, 2022 19:59

JelleZijlstra reviewed Apr 17, 2022
View reviewed changes

Comment threadMisc/NEWS.d/next/Core and Builtins/2022-03-08-21-59-57.bpo-46962.UomDfz.rst Outdated
Comment threadMisc/NEWS.d/next/Core and Builtins/2022-03-08-21-59-57.bpo-46962.UomDfz.rst Outdated

JelleZijlstra self-assigned this Apr 17, 2022

Wordsmith the NEWS entry
c4a5025

JelleZijlstra approved these changes Apr 17, 2022
View reviewed changes

bedevere-bot added awaiting merge and removed awaiting review labels Apr 17, 2022

JelleZijlstra merged commit a573cb2 into python:mainApr 18, 2022

arhadthedev mentioned this pull request Apr 10, 2022
Fix docstrings that do not honor --without-doc-strings #91118
Closed

bedevere-bot removed the awaiting merge label Apr 18, 2022

JelleZijlstra added needs backport to 3.9 needs backport to 3.10 only security fixes labels Apr 18, 2022

arhadthedev deleted the ctypes-without-doc-string branch April 18, 2022 03:45

bedevere-bot removed the needs backport to 3.10 only security fixes label Apr 18, 2022

bedevere-bot removed the needs backport to 3.9 label Apr 18, 2022

	:class:`~_ctypes.PyCPointerType:, :class:`~_ctypes.PyCSimpleType:, and
	:class:`~_ctypes.PyCPointerType`, :class:`~_ctypes.PyCSimpleType`, and

	:class:`~_ctypes.PyCStructType:.
	:class:`~_ctypes.PyCStructType`.

Uh oh!

Uh oh!

Conversation

arhadthedev commented Mar 8, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JelleZijlstra left a comment

Choose a reason for hiding this comment

Uh oh!

JelleZijlstraMar 8, 2022

Choose a reason for hiding this comment

Uh oh!

JelleZijlstraMar 8, 2022

Choose a reason for hiding this comment

Uh oh!

arhadthedevMar 9, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

miss-islington commented Apr 18, 2022

Uh oh!

miss-islington commented Apr 18, 2022

Uh oh!

miss-islington commented Apr 18, 2022

Uh oh!

miss-islington commented Apr 18, 2022

Uh oh!

JelleZijlstra commented Apr 18, 2022

Uh oh!

bedevere-bot commented Apr 18, 2022

Uh oh!

bedevere-bot commented Apr 18, 2022

Uh oh!

arhadthedev commented Apr 18, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

arhadthedev commented Mar 8, 2022•
edited
Loading