[PATCH 0 of 2 RFC] Introduce defaultdict in mercurial?

Nicolas Dumazet nicdumz at gmail.com
Fri Aug 21 10:43:10 CDT 2009


Hello -devel!

I find defaultdict, the Python 2.5 feature, very useful and quite elegant in
many cases. I am trying today to introduce this feature in mercurial.

Since we still support Python 2.4, I introduce in util a simple wrapper around
dict to emulate defaultdict for Python 2.4 users. >= 2.5 users have a native,
C implementation of defaultdicts.
Any thoughts about this?


In our codebase we have sometimes cases where setdefault is used at
initialization:

    for p in ...:
        children.setdefault(p, []).append(n)

But then, later, for some reason we still have to use get() because not
every key seems safe:

    for c in children.get(n, []):
        foo()

I think that in those case in particular, it's cleaner and simpler to 
declare children as a defaultdict(list):

    children = util.defaultdict(list)
    # ...
    for p in ...:
        children[p].append(n)

    # ...
    for c in children[n]:
        foo()

Do you agree too that in some cases it is helpful and cleaner to use
defaultdict?

The second patch attempts to replace usage of dict+setdefault by a defaultdict,
where relevant. Do you agree with my choices?

I left alone some cases. For example, in locarepo.py:

    # Create the set of filenodes for the file if
    # there isn't one already.
    ndset = msng_filenode_set.setdefault(f, {})
    # And set the filenode's changelog node to the
    # manifest's if it hasn't been set already.
    ndset.setdefault(fnode, clnode)

The two lines could be changed to msng_filenode_set[f].setdefault(fnode, clnode)
but at the sake of readability. I'd rather have longer and more readable code
where needed.


(As a side question: what guidelines are there for submitting rather long
refactoring patches? I understand that the second patch is probably quite long
and hard to review; does it makes sense to split the commit per filegroups,
maybe?)


Thanks,
Nicolas.


More information about the Mercurial-devel mailing list