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

defaultdict.fromkeys should accept a callable factory #67561

Closed
justanr mannequin opened this issue Feb 1, 2015 · 9 comments
Closed

defaultdict.fromkeys should accept a callable factory #67561

justanr mannequin opened this issue Feb 1, 2015 · 9 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@justanr
Copy link
Mannequin

justanr mannequin commented Feb 1, 2015

BPO 23372
Nosy @rhettinger, @allenap, @serhiy-storchaka, @vedgar

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 2016-09-20.03:39:26.339>
created_at = <Date 2015-02-01.16:19:44.617>
labels = ['type-bug', 'library']
title = 'defaultdict.fromkeys should accept a callable factory'
updated_at = <Date 2016-09-20.03:39:26.338>
user = 'https://bugs.python.org/justanr'

bugs.python.org fields:

activity = <Date 2016-09-20.03:39:26.338>
actor = 'rhettinger'
assignee = 'rhettinger'
closed = True
closed_date = <Date 2016-09-20.03:39:26.339>
closer = 'rhettinger'
components = ['Library (Lib)']
creation = <Date 2015-02-01.16:19:44.617>
creator = 'justanr'
dependencies = []
files = []
hgrepos = []
issue_num = 23372
keywords = []
message_count = 9.0
messages = ['235180', '235181', '235187', '235191', '235193', '235196', '276981', '276997', '277000']
nosy_count = 6.0
nosy_names = ['rhettinger', 'allenap', 'SilentGhost', 'serhiy.storchaka', 'veky', 'justanr']
pr_nums = []
priority = 'normal'
resolution = 'rejected'
stage = None
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue23372'
versions = ['Python 3.5']

@justanr
Copy link
Mannequin Author

justanr mannequin commented Feb 1, 2015

Not something I've noticed until today, but defaultdict.fromkeys only accepts an iterable of keys and an initial value. To set the default_factory for the newly created collection, you need to prod at the attribute itself.

This isn't a huge issue for me, however adding an optional default_factory to fromkeys would be a nice convenience.

@justanr justanr mannequin added the type-bug An unexpected behavior, bug, or error label Feb 1, 2015
@SilentGhost
Copy link
Mannequin

SilentGhost mannequin commented Feb 1, 2015

Correct me if I'm wrong but this seem as a very unlikely use case. Why would you need to ensure content of the defaultdict (i.e., why would you ever use its fromkeys method)? And, perhaps, having to implicitly assign default factory is not such a bad tradeoff, considering implementation overhead.

@SilentGhost SilentGhost mannequin added the stdlib Python modules in the Lib dir label Feb 1, 2015
@rhettinger
Copy link
Contributor

-0 I could see some use cases for this, dict.fromkeys(contestants, factory=list), but it adds complexity to a core container API and it starts to overlap the use cases for collections.defaultdict().

Perhaps set comprehensions should remain the one-obvious-way-to-do-it:
{k : [] for k in contestants}

@rhettinger rhettinger self-assigned this Feb 1, 2015
@SilentGhost
Copy link
Mannequin

SilentGhost mannequin commented Feb 1, 2015

Raymond, but Alec talks about

  defaultdict.fromkeys(constants, factory=list)

as opposed to current solution

  d = defaultdict.fromkeys(constants)
  d.default_factory = list
  for i in d:
      d[i] = []

I wouldn't think that the dict.fromkeys should be affected by this issue.

@justanr
Copy link
Mannequin Author

justanr mannequin commented Feb 1, 2015

@SilentGhost I was playing with a voting algorithm (Instant Runoff, rather than traditional majority). Ultimately, I ended up using Counter in place of a defaultdict, but that's how I ended up noticing it.

Not so much ensuring the content so much as setting an initial default state to mutate.

@rhettinger I had considered a comprehension and passing it into a defaultdict (or Counter) to get that nice missing key handling.

Implicitly assigning the factory is a pretty good compromise, like I said fromkeys accepting the factory would be a nice convenience rather than correcting a major oversight.

@serhiy-storchaka
Copy link
Member

It may be written simpler:

    d = defaultdict(factory)
    for i in constants:
        d[i]

or

    d = defaultdict(factory, {i: factory() for i in constants})

I'm inclined to reject this proposition. It serves very special use case.

@allenap
Copy link
Mannequin

allenap mannequin commented Sep 19, 2016

It's inconsistent that defaultdict([]) should be rejected:

  >>> defaultdict([])
  Traceback (most recent call last):
    File "<stdin>", line 1, in <module>
  TypeError: first argument must be callable or None

but defaultdict.fromkeys([]) is okay:

  >>> defaultdict.fromkeys([])
  defaultdict(None, {})

The constructor signature differs between defaultdict and dict, and defaultdict.fromkeys is an alternate constructor, so it seems reasonable to also change its signature.

Also confusing is that I can call fromkeys on an instance of defaultdict:

  >>> dd = defaultdict(list)
  >>> dd.fromkeys([1])
  defaultdict(None, {1: None})

Instinctively I expect the default_factory to be carried over, even though I realise that would be odd behaviour for Python. If defaultdict.fromkeys were to expect a mandatory default_factory argument there wouldn't be this moment of confusion.

@vedgar
Copy link
Mannequin

vedgar mannequin commented Sep 20, 2016

That's usual behavior for any class method. You can call them on an instance, but they don't have the access to it, only to its class. So transferring of factory would in fact not be possible. Of course,it's possible to make fromkeys an instance method, but please don't do that.

@rhettinger
Copy link
Contributor

[Serhiy]
I'm inclined to reject this proposition. It serves very special use case.

[Silent Ghost]
Correct me if I'm wrong but this seem as a very unlikely use case

[Alec Nikolas Reiter]
Implicitly assigning the factory is a pretty good compromise, like I said fromkeys accepting the factory would be a nice convenience rather than correcting a major oversight.

-----

After thinking about the above, I'm disinclined to make the API modification.

The current way to do it is straight-forward and reads nicely:

>>> d = defaultdict.fromkeys('abcdefg', somedefault)
>>> d.default_factory = somefunc

Combining those into a single step is less readable and may incorrectly imply some sort of interaction between the factory function and initial population of keys with a constant default value. Likewise, the possibility of implicitly guessing and assigning the factory function is contraindicated by the zen-of-python in at least two places.

Given the paucity of use cases, the potential for API confusion, and the ready availability of reasonable alternatives, I'm closing the feature request. Thank for the suggestion though, it certainly seemed like something that deserved a little more thought.

@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
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

2 participants