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 of OrderedDict #73270

Closed
serhiy-storchaka opened this issue Dec 27, 2016 · 7 comments
Closed

C API of OrderedDict #73270

serhiy-storchaka opened this issue Dec 27, 2016 · 7 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs)

Comments

@serhiy-storchaka
Copy link
Member

BPO 29084
Nosy @brettcannon, @rhettinger, @ericsnowcurrently, @serhiy-storchaka
PRs
  • bpo-29084: Exclude C API for OrderedDict from the limited C API. #4900
  • [3.6] bpo-29084: Exclude C API for OrderedDict from the limited C API. (GH-4900) #5007
  • 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 = 'https://github.com/serhiy-storchaka'
    closed_at = <Date 2017-12-25.07:41:02.690>
    created_at = <Date 2016-12-27.13:59:17.756>
    labels = ['interpreter-core']
    title = 'C API of OrderedDict'
    updated_at = <Date 2017-12-25.07:41:02.689>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2017-12-25.07:41:02.689>
    actor = 'serhiy.storchaka'
    assignee = 'serhiy.storchaka'
    closed = True
    closed_date = <Date 2017-12-25.07:41:02.690>
    closer = 'serhiy.storchaka'
    components = ['Interpreter Core']
    creation = <Date 2016-12-27.13:59:17.756>
    creator = 'serhiy.storchaka'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 29084
    keywords = ['patch']
    message_count = 7.0
    messages = ['284098', '284115', '284175', '286014', '308463', '309020', '309030']
    nosy_count = 4.0
    nosy_names = ['brett.cannon', 'rhettinger', 'eric.snow', 'serhiy.storchaka']
    pr_nums = ['4900', '5007']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue29084'
    versions = []

    @serhiy-storchaka
    Copy link
    Member Author

    C API of the C implementation of OrderedDict was added in 3.5 together with the C implementation of OrderedDict. But it was never announced nor documented. Some macros contain bugs. PyODict_Check() and PyODict_CheckExact() are declared in the limited API, but don't work since use PyODict_Type not available in the limited API. PyODict_SIZE() is expanded to syntactically incorrect code and it's name is not consistent with similar macros: PyTuple_GET_SIZE, PyList_GET_SIZE and just added PyDict_GET_SIZE. Many names are just transparent wrappers around PyDict API (and they can't be different).

    Since PyODict C API is not documented, partially not working and partially redundant, and the C implementation of OrderedDict is optional (other Python implementations can provide just Python implementation of OrderedDict), I think this C API should be made private if not removed.

    @serhiy-storchaka serhiy-storchaka added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Dec 27, 2016
    @brettcannon
    Copy link
    Member

    If the API is broken in the limited API then I definitely think we should consistently make it not part of the limited API. As for removing it I don't have an opinion without knowing how many people are using it.

    @rhettinger
    Copy link
    Contributor

    +1 for privatizing

    @serhiy-storchaka
    Copy link
    Member Author

    Eric, what are your thoughts?

    @serhiy-storchaka serhiy-storchaka self-assigned this Oct 23, 2017
    @rhettinger
    Copy link
    Contributor

    With today's announcement of guaranteed ordering for regular dicts, OrderedDict just became much less important (almost insignificant). Also, the original reason for having this C API no longer applies.

    There is a reasonable case to be made for removing the API entirely (given that it was never documented, that it is buggy, and that it is no longer of any significance).

    @serhiy-storchaka
    Copy link
    Member Author

    New changeset 1b3029a by Serhiy Storchaka in branch 'master':
    bpo-29084: Exclude C API for OrderedDict from the limited C API. (bpo-4900)
    1b3029a

    @serhiy-storchaka
    Copy link
    Member Author

    New changeset d62b741 by Serhiy Storchaka (Miss Islington (bot)) in branch '3.6':
    bpo-29084: Exclude C API for OrderedDict from the limited C API. (GH-4900) (bpo-5007)
    d62b741

    @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
    interpreter-core (Objects, Python, Grammar, and Parser dirs)
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants