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

C API doc for PyObject_HEAD is outdated #67898

Closed
tpievila mannequin opened this issue Mar 19, 2015 · 10 comments
Closed

C API doc for PyObject_HEAD is outdated #67898

tpievila mannequin opened this issue Mar 19, 2015 · 10 comments
Labels
docs Documentation in the Doc dir easy type-bug An unexpected behavior, bug, or error

Comments

@tpievila
Copy link
Mannequin

tpievila mannequin commented Mar 19, 2015

BPO 23710
Nosy @bitdancer, @berkerpeksag, @matrixise, @evelynmitchell, @ammaraskar
Files
  • capidocs.diff
  • capidocs.diff-2
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2016-08-03.09:59:23.429>
    created_at = <Date 2015-03-19.14:41:38.985>
    labels = ['easy', 'type-bug', 'docs']
    title = 'C API doc for PyObject_HEAD is outdated'
    updated_at = <Date 2016-08-03.09:59:23.428>
    user = 'https://bugs.python.org/tpievila'

    bugs.python.org fields:

    activity = <Date 2016-08-03.09:59:23.428>
    actor = 'berker.peksag'
    assignee = 'docs@python'
    closed = True
    closed_date = <Date 2016-08-03.09:59:23.429>
    closer = 'berker.peksag'
    components = ['Documentation']
    creation = <Date 2015-03-19.14:41:38.985>
    creator = 'tpievila'
    dependencies = []
    files = ['43649', '43657']
    hgrepos = []
    issue_num = 23710
    keywords = ['patch', 'easy']
    message_count = 10.0
    messages = ['238530', '263735', '269912', '269936', '269955', '269956', '271858', '271860', '271886', '271887']
    nosy_count = 8.0
    nosy_names = ['r.david.murray', 'docs@python', 'python-dev', 'berker.peksag', 'tpievila', 'matrixise', 'Evelyn Mitchell', 'ammar2']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue23710'
    versions = ['Python 3.5', 'Python 3.6']

    @tpievila
    Copy link
    Mannequin Author

    tpievila mannequin commented Mar 19, 2015

    https://docs.python.org/3/c-api/structures.html#c.PyObject_HEAD says that the macro should be used for creating new types and that it has ob_type. Unfortunately at some point (@61466 perhaps) that definition in object.h was changed. This affects other pages too (at least https://docs.python.org/3/extending/newtypes.html).

    I encountered this while checking if a C extension (imposm.parser) could be trivially ported for Python 3. Unfortunately not. I suppose

    self->ob_type

    should be replaced with

      Py_TYPE(self)

    but that isn't really clear from the docs.

    @tpievila tpievila mannequin assigned docspython Mar 19, 2015
    @tpievila tpievila mannequin added the docs Documentation in the Doc dir label Mar 19, 2015
    @berkerpeksag
    Copy link
    Member

    Docs of PyObject_HEAD and friends have already been fixed in 760c5cfacbaa.

    https://docs.python.org/3/extending/newtypes.html still needs to be updated to mention ob_base:

    This is what a Noddy object will contain—in this case, nothing more than what every Python object contains—a refcount and a pointer to a type object. These are the fields the PyObject_HEAD macro brings in.
    

    @berkerpeksag berkerpeksag added easy type-bug An unexpected behavior, bug, or error labels Apr 19, 2016
    @ammaraskar
    Copy link
    Member

    This diff amends the documentation to point out that PyObject_HEAD includes the ob_base field, gives a brief overview of it and points to the PyObject/structures page for more information.

    It also changes the semicolon part to a warning for clarity.

    @bitdancer
    Copy link
    Member

    Warnings are big red boxes, and we try to only use those for really critical info. The current text is what we want as far as that goes.

    @ammaraskar
    Copy link
    Member

    The current text is what we want as far as that goes.

    Roger that, I've reverted the change to make that a warning in the newest diff.

    @bitdancer
    Copy link
    Member

    Looks good to me.

    @evelynmitchell
    Copy link
    Mannequin

    evelynmitchell mannequin commented Aug 2, 2016

    There is a patch, and it looks good to me. This should be moved into 'commit review' state.

    @evelynmitchell
    Copy link
    Mannequin

    evelynmitchell mannequin commented Aug 2, 2016

    Changed to commit review.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Aug 3, 2016

    New changeset 92b468020e07 by Berker Peksag in branch '3.5':
    Issue bpo-23710: Update PyObject_HEAD documentation
    https://hg.python.org/cpython/rev/92b468020e07

    New changeset 26dd6e7f1733 by Berker Peksag in branch 'default':
    Issue bpo-23710: Merge from 3.5
    https://hg.python.org/cpython/rev/26dd6e7f1733

    @berkerpeksag
    Copy link
    Member

    Thanks for the patch. Committed capidocs.diff-2 with some tweaks.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    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 easy type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants