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

dict.fromkeys() #37531

Closed
rhettinger opened this issue Nov 25, 2002 · 14 comments
Closed

dict.fromkeys() #37531

rhettinger opened this issue Nov 25, 2002 · 14 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs)

Comments

@rhettinger
Copy link
Contributor

BPO 643443
Nosy @gvanrossum, @theller, @rhettinger
Files
  • sequp.diff: Patch with code, docs, tests, and news
  • fromseq.diff: Static method version
  • fromseq2.diff: Class method version
  • fromseq3.diff: Class method version with fixed new item
  • fromseq4.diff: Class method version with Just's comments
  • 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/rhettinger'
    closed_at = <Date 2002-11-27.07:36:15.000>
    created_at = <Date 2002-11-25.10:29:38.000>
    labels = ['interpreter-core']
    title = 'dict.fromkeys()'
    updated_at = <Date 2002-11-27.07:36:15.000>
    user = 'https://github.com/rhettinger'

    bugs.python.org fields:

    activity = <Date 2002-11-27.07:36:15.000>
    actor = 'rhettinger'
    assignee = 'rhettinger'
    closed = True
    closed_date = None
    closer = None
    components = ['Interpreter Core']
    creation = <Date 2002-11-25.10:29:38.000>
    creator = 'rhettinger'
    dependencies = []
    files = ['4731', '4732', '4733', '4734', '4735']
    hgrepos = []
    issue_num = 643443
    keywords = ['patch']
    message_count = 14.0
    messages = ['41753', '41754', '41755', '41756', '41757', '41758', '41759', '41760', '41761', '41762', '41763', '41764', '41765', '41766']
    nosy_count = 4.0
    nosy_names = ['gvanrossum', 'theller', 'rhettinger', 'jvr']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = None
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue643443'
    versions = ['Python 2.3']

    @rhettinger
    Copy link
    Contributor Author

    Implements dict.sequpdate() as discussed on python-
    dev:

    def sequpdate(self, iterable, value=True):
        for k in iterable:
            self[k] = value
        return self

    @rhettinger rhettinger self-assigned this Nov 25, 2002
    @rhettinger rhettinger added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Nov 25, 2002
    @rhettinger rhettinger self-assigned this Nov 25, 2002
    @rhettinger rhettinger added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Nov 25, 2002
    @jvr
    Copy link
    Mannequin

    jvr mannequin commented Nov 25, 2002

    Logged In: YES
    user_id=92689

    I see no reason why d.sequpdate() should return self.
    d.update() doesn't either.

    Also, as I suggested on python-dev, a fromseq() constructor
    (as a class method) would be nice:

      >>> dict.fromseq('End Quit Stop Abort'.split())
      {'End': True, 'Quit': True, 'Stop': True, 'Abort': True}
      >>>

    @jvr
    Copy link
    Mannequin

    jvr mannequin commented Nov 25, 2002

    Logged In: YES
    user_id=92689

    One more thing: I'd say the default value should be None.
    This makes the idiom also useful to initialize dicts which
    later get filled with proper values. This doesn't take
    anything away from your original use case.

    @rhettinger
    Copy link
    Contributor Author

    Logged In: YES
    user_id=80475

    Attached revised patch incorporating all of the review
    comments. Changed default to None. Made a static
    method instead of a classmethod because cls was not
    used. Doesn't return self.

    Tests, docs, and news item revised accordingly.

    Ready for BDFL pronouncement.

    @gvanrossum
    Copy link
    Member

    Logged In: YES
    user_id=6380

    Hm, but the convention for "alternate constructors" is that
    the cls argument *is* used to determine the type of the new
    instance. This way, the constructors are inherited in a
    meaningful way by subclasses of dict.

    @rhettinger
    Copy link
    Contributor Author

    Logged In: YES
    user_id=80475

    Done! Revise patch attached.

    It is now a class method and uses cls for the type of the
    new instance. Added tests to demonstrate meaningful
    inheritance.

    @theller
    Copy link

    theller commented Nov 26, 2002

    Logged In: YES
    user_id=11105

    It seems the code and the docs are out of sync in
    fromseq2.diff.

    The docs say "fromseq(sequence, value=True)" while the
    code does "fromseq(sequence, value=None)"

    @rhettinger
    Copy link
    Contributor Author

    Logged In: YES
    user_id=80475

    Fixed news item.

    @jvr
    Copy link
    Mannequin

    jvr mannequin commented Nov 26, 2002

    Logged In: YES
    user_id=92689

    • I'm not sure whether PyObject_CallObject() is the right
      idiom here. I would have expected to call cls->tp_new (if
      not NULL), but I'm not sure either way.
    • I would get rid of the loop counter: you're not using it.
      for (;;) will do nicely
    • you're XDECREF'ing values of which you know they're not NULL
    • idem XINCREF. Also: I'm pretty sure you should incref d at
      the end: I think it currently leaks.

    @jvr
    Copy link
    Mannequin

    jvr mannequin commented Nov 26, 2002

    Logged In: YES
    user_id=92689

    dang, that should've read "you should NOT incref d at the end".

    @rhettinger
    Copy link
    Contributor Author

    Logged In: YES
    user_id=80475

    Okay, fixed decrefs, eliminated i; used for(;;).
    Left in PyObject_CallObject because it's clean.
    Thanks.

    @jvr
    Copy link
    Mannequin

    jvr mannequin commented Nov 26, 2002

    Logged In: YES
    user_id=92689

    Btw. I was looking for more info on METH_CLASS and came
    across this thread on python-dev:

    http://mail.python.org/pipermail/python-dev/2002-April/023566.html
    which resulted in this bug report:
    http://www.python.org/sf/548651
    which has not been resolve. I'll add a comment there, too.

    @gvanrossum
    Copy link
    Member

    Logged In: YES
    user_id=6380

    I'm ok with this, but I haven't had time to review the code.

    @rhettinger
    Copy link
    Contributor Author

    Logged In: YES
    user_id=80475

    After more code review, added PyDict_Check.

    Also, discussion on python-dev showed a need to rename
    the method to indicate that keys were being used rather
    than items. The sequence part of the name was dropped
    because any iterable will do as an argument.

    Checked-in as:
    Misc/NEWS 1.548
    Objects/dictobject.c 2.132
    Lib/test/test_types.py 1.41
    Doc/lib/libstdtypes.tex 1.111

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 9, 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