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

Jump to bottom

Always return a list from PyMapping_Keys/PyMapping_Values/PyMapping_Items #72467

Closed
serhiy-storchaka opened this issue Sep 26, 2016 · 5 comments
Closed

Always return a list from PyMapping_Keys/PyMapping_Values/PyMapping_Items #72467

serhiy-storchaka opened this issue Sep 26, 2016 · 5 comments
Labels
3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@serhiy-storchaka
Copy link
Member

BPO 28280
Nosy @rhettinger, @serhiy-storchaka, @zhangyangyu, @orenmn
PRs
  • bpo-28280: Make PyMapping_Keys(), PyMapping_Values() and PyMapping_Items() always return a list #3840
    		•

    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 2017-10-21.17:18:52.387>
created_at = <Date 2016-09-26.18:13:46.369>
labels = ['interpreter-core', 'type-feature', '3.7']
title = 'Always return a list from PyMapping_Keys/PyMapping_Values/PyMapping_Items'
updated_at = <Date 2017-10-21.17:18:52.386>
user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2017-10-21.17:18:52.386>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = True
closed_date = <Date 2017-10-21.17:18:52.387>
closer = 'serhiy.storchaka'
components = ['Interpreter Core']
creation = <Date 2016-09-26.18:13:46.369>
creator = 'serhiy.storchaka'
dependencies = []
files = []
hgrepos = []
issue_num = 28280
keywords = ['patch']
message_count = 5.0
messages = ['277443', '303340', '303348', '303413', '303902']
nosy_count = 4.0
nosy_names = ['rhettinger', 'serhiy.storchaka', 'xiang.zhang', 'Oren Milman']
pr_nums = ['3840']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue28280'
versions = ['Python 3.7']
    @serhiy-storchaka
    Copy link
    Member Author

    PyMapping_Keys(), PyMapping_Values() and PyMapping_Items() can return a list or tuple (because they use PySequence_Fast). But end users of this API often work only with lists and raise an exception if corresponding mapping methods return an instance of other type (actually only for tuples). See Modules/posixmodule.c, Modules/_winapi.c, Modules/_sre.c. The type is even not checked in the latter file.

    Old documentation said that PyMapping_Keys(o) is equivalent to the Python expression list(o.keys()). Maybe it is worth to make this true. Make PyMapping_Keys(o) etc always returning a list. This could simplify the code that uses this C API. Since keys() etc usually return a dict view, a list or a generator, but not a tuple, this unlikely will cause performance regression.

    @serhiy-storchaka serhiy-storchaka added 3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement labels Sep 26, 2016
    @orenmn
    Copy link
    Mannequin

    orenmn mannequin commented Sep 29, 2017

    I would be happy to write a PR that implements that.

    However, i am not sure which way is better to construct a list from the return
    value (an iterable, hopefully) of keys() etc.:

    • Call PyList_Type() (in each of PyMapping_Keys() etc.) on the iterable, and
      overwrite the error message in case it is a TypeError.
    • Write a helper function iterable_as_list(), which uses PyObject_GetIter() and
      PySequence_List(), and call it in each of PyMapping_Keys() etc..
      (iterable_as_list() would receive "keys" etc., so that it would raise the
      appropriate error message, in case of a TypeError.)

    ISTM that the first one is simpler, but I am not sure about the performance
    difference between them.

    @serhiy-storchaka
    Copy link
    Member Author

    PySequence_List() is the simplest way in C. I don't know whether we need special error messages now. But for compatibility keep them.

    @orenmn
    Copy link
    Mannequin

    orenmn mannequin commented Sep 30, 2017

    (for knowledge preservation's sake)
    Resolving this issue would also resolve bpo-31486.

    @serhiy-storchaka
    Copy link
    Member Author

    New changeset 0ccc0f6 by Serhiy Storchaka (Oren Milman) in branch 'master':
    bpo-28280: Make PyMapping_Keys(), PyMapping_Values() and PyMapping_Items() always return a list (bpo-3840)
    0ccc0f6

    @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
    Assignees
    No one assigned
    Labels
    3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    1 participant
    @serhiy-storchaka