Skip to content
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

Merged
merged 43 commits into from
Jun 14, 2018

Conversation

ericsnowcurrently
Copy link
Member

@ericsnowcurrently ericsnowcurrently commented Jun 5, 2018

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

@ericsnowcurrently ericsnowcurrently added the docs Documentation in the Doc dir label Jun 5, 2018
PyTypeObject Slot [#slots]_ Inherited [#inh]_ Type
=========================================== ================= ======================
:c:member:`~PyTypeObject.tp_name` .. char *
:c:member:`~PyTypeObject.tp_basicsize` X int
Copy link
Member

Choose a reason for hiding this comment

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

Py_ssize_t

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

=========================================== ================= ======================
PyTypeObject Slot [#slots]_ Inherited [#inh]_ Type
=========================================== ================= ======================
:c:member:`~PyTypeObject.tp_name` .. char *
Copy link
Member

Choose a reason for hiding this comment

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

const char *tp_name

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

: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 *
Copy link
Member

Choose a reason for hiding this comment

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

const char *

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

=========================================== ================= ======================
:c:member:`~PyTypeObject.tp_name` .. char *
:c:member:`~PyTypeObject.tp_basicsize` X int
:c:member:`~PyTypeObject.tp_itemsize` ? int
Copy link
Member

Choose a reason for hiding this comment

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

Py_ssize_t

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

: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
Copy link
Member

Choose a reason for hiding this comment

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

unsigned long

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@@ -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::
Copy link
Member

Choose a reason for hiding this comment

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

Described in structures.rst.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed


**Default:**

PyBaseObject_Type provides a tp_richcompare. However, if only
Copy link
Member

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

**Default:**

This slot has no default. For static types, if the field is
NULL then no __dict__ gets created for instances.
Copy link
Member

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__.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

:c:func:`PyType_GenericAlloc`, to force a standard heap
allocation strategy.

For static subtypess, PyBaseObject_Type uses
Copy link
Member

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, subtypess -> subtypes

Copy link
Member Author

Choose a reason for hiding this comment

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

typo fixed

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

**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
Copy link
Member

Choose a reason for hiding this comment

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

Add markup for NULL.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@serhiy-storchaka
Copy link
Member

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.

@ericsnowcurrently
Copy link
Member Author

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.

@ericsnowcurrently
Copy link
Member Author

Ah, I need to run "make suspicious". :/

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*.
Copy link
Member

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?

Copy link
Member Author

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.

Copy link
Member Author

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.


.. code-block:: none

X - *PyType_Ready* always sets this value (if *NULL*)
Copy link
Member

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”?

Copy link
Member Author

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.


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*.
Copy link
Member

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

Copy link
Member Author

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.


**Default:**

:c:type:`PyBaseObject_Type` provides a :attr:`tp_richcompare`.
Copy link
Member

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”

Copy link
Member Author

Choose a reason for hiding this comment

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

agreed

@ericsnowcurrently
Copy link
Member Author

I've addressed all review comments.

@ericsnowcurrently ericsnowcurrently merged commit 9e7c921 into python:master Jun 14, 2018
@ericsnowcurrently ericsnowcurrently deleted the typeobj-doc branch June 14, 2018 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants