-
-
Notifications
You must be signed in to change notification settings - Fork 30.9k
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
bpo-17045: Improve C-API doc for PyTypeObject. #7413
bpo-17045: Improve C-API doc for PyTypeObject. #7413
Conversation
Doc/c-api/typeobj.rst
Outdated
PyTypeObject Slot [#slots]_ Inherited [#inh]_ Type | ||
=========================================== ================= ====================== | ||
:c:member:`~PyTypeObject.tp_name` .. char * | ||
:c:member:`~PyTypeObject.tp_basicsize` X int |
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.
Py_ssize_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.
fixed
Doc/c-api/typeobj.rst
Outdated
=========================================== ================= ====================== | ||
PyTypeObject Slot [#slots]_ Inherited [#inh]_ Type | ||
=========================================== ================= ====================== | ||
:c:member:`~PyTypeObject.tp_name` .. char * |
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.
const char *tp_name
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.
fixed
Doc/c-api/typeobj.rst
Outdated
:c:member:`~PyTypeObject.tp_setattro` X :c:type:`setattrofunc` | ||
:c:member:`~PyTypeObject.tp_as_buffer` \* PyBufferProcs * | ||
:c:member:`~PyTypeObject.tp_flags` ? long | ||
:c:member:`~PyTypeObject.tp_doc` .. char * |
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.
const char *
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.
fixed
Doc/c-api/typeobj.rst
Outdated
=========================================== ================= ====================== | ||
:c:member:`~PyTypeObject.tp_name` .. char * | ||
:c:member:`~PyTypeObject.tp_basicsize` X int | ||
:c:member:`~PyTypeObject.tp_itemsize` ? int |
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.
Py_ssize_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.
fixed
Doc/c-api/typeobj.rst
Outdated
:c:member:`~PyTypeObject.tp_getattro` X :c:type:`getattrofunc` | ||
:c:member:`~PyTypeObject.tp_setattro` X :c:type:`setattrofunc` | ||
:c:member:`~PyTypeObject.tp_as_buffer` \* PyBufferProcs * | ||
:c:member:`~PyTypeObject.tp_flags` ? long |
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.
unsigned long
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.
fixed
Doc/c-api/typeobj.rst
Outdated
@@ -732,19 +1025,45 @@ type objects) *must* have the :attr:`ob_size` field. | |||
For each entry in the array, an entry is added to the type's dictionary (see | |||
:c:member:`~PyTypeObject.tp_dict` below) containing a getset descriptor. | |||
|
|||
.. XXX belongs elsewhere | |||
|
|||
Docs for PyGetSetDef:: |
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.
Described in structures.rst
.
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.
fixed
Doc/c-api/typeobj.rst
Outdated
|
||
**Default:** | ||
|
||
PyBaseObject_Type provides a tp_richcompare. However, if only |
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.
Add markup for PyBaseObject_Type
and tp_richcompare
.
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.
fixed
Doc/c-api/typeobj.rst
Outdated
**Default:** | ||
|
||
This slot has no default. For static types, if the field is | ||
NULL then no __dict__ gets created for instances. |
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.
Add markup for NULL
and __dict__
.
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.
fixed
Doc/c-api/typeobj.rst
Outdated
:c:func:`PyType_GenericAlloc`, to force a standard heap | ||
allocation strategy. | ||
|
||
For static subtypess, PyBaseObject_Type uses |
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.
Add markup for PyBaseObject_Type
.
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.
Also, subtypess -> subtypes
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.
typo fixed
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.
fixed
Doc/c-api/typeobj.rst
Outdated
**Default:** | ||
|
||
For static types this field has no default. This means if the | ||
slot is defined as NULL, the type cannot be called to create new |
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.
Add markup for NULL
.
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.
fixed
I'm not sure about usefulness of Default: notes. If the slot is inherited, then the default is the parent's value. It looks to me that Inheritane: notes cover this, and Default: notes don't add new information. |
If I recall correctly, there are a few slots with defaults. Would it be better to omit the "default" section in the rest of the cases? FWIW, I added the "default" section because the defaults aren't always clear. |
722cc7f
to
3b7c312
Compare
Ah, I need to run "make suspicious". :/ |
Doc/c-api/typeobj.rst
Outdated
A slot name in parentheses indicates it is (effectively) deprecated. | ||
Names in angle brackets should be treated as read-only. | ||
Names in square brackets are for internal use only. | ||
An asterisk means the field must be non-*NULL*. |
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.
Can you find another notation that doesn’t look like a pointer type or dereferencing a pointer?
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.
Sure. I was following the existing convention in the file, but agree it's less clear.
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.
Oh, you're talking about the notation in the table. :) I'll change it to something else.
Doc/c-api/typeobj.rst
Outdated
|
||
.. code-block:: none | ||
|
||
X - *PyType_Ready* always sets this value (if *NULL*) |
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.
The always and brackets confuse me. Would this be more correct: “PyTypeReady sets this value if it is NULL”?
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.
I agree that it's not as clear as I want. My goal was to emphasize that it does get set by PyType_Ready(). I'll make it more clear.
Doc/c-api/typeobj.rst
Outdated
|
||
This field is inherited by subtypes together with :c:member:`~PyTypeObject.tp_setattr`: a subtype | ||
inherits both :c:member:`~PyTypeObject.tp_setattr` and :c:member:`~PyTypeObject.tp_setattro` from its base type when | ||
the subtype's :c:member:`~PyTypeObject.tp_setattr` and :c:member:`~PyTypeObject.tp_setattro` are both *NULL*. | ||
THE SUBTYpe's :c:member:`~PyTypeObject.tp_setattr` and :c:member:`~PyTypeObject.tp_setattro` are both *NULL*. |
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 line looks like a mistake
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.
Yep. I'll fix it.
Doc/c-api/typeobj.rst
Outdated
|
||
**Default:** | ||
|
||
:c:type:`PyBaseObject_Type` provides a :attr:`tp_richcompare`. |
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.
Drop a or add a real noun, i.e. “provides tp_richcompare” or “provides a tp_richcompare implementation”
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.
agreed
I've addressed all review comments. |
This is a quick port to git of a patch I wrote for the issue several years ago. I'm sure I have it mostly okay, but haven't had time yet to check it closely.
https://bugs.python.org/issue17045