[PATCH 03 of 15 v2] dirstate: create class for status lists

Yuya Nishihara yuya at tcha.org
Tue Oct 7 07:05:45 CDT 2014


On Mon, 06 Oct 2014 20:20:07 +0200, Mads Kiilerich wrote:
> On 10/05/2014 08:08 AM, Martin von Zweigbergk wrote:
> > +class status(tuple):
> > +    '''Named tuple with a list of files per status.
> > +    '''
> 
> Hmm. There are several things I would like to point out ... but they 
> seem to all be inherited from the Python namedtuple. It might make sense 
> to stay as similar as possible but slightly different.
> 
> Why not use a generic named tuple implementation ... especially on 
> Python 2.6+ where it is built-in? There might be good reasons, but they 
> should documented.
> 
> I guess one reason is that the standard implementation creates the class 
> as source code and execs it. -1 for elegance, +1 for getting the work done.
> 
> Another reason is that this implementation defaults to having 
> unspecified parameters as empty lists ... but is that really relevant / 
> necessary?
> 
> > +
> > +    __slots__ = ()
> > +
> > +    def __new__(cls, modified=[], added=[], removed=[], deleted=[], unknown=[],
> > +                ignored=[], clean=[]):
> 
> http://docs.python-guide.org/en/latest/writing/gotchas/#mutable-default-arguments 
> - this is the only significant issue I see with this series.
> 
> > +        return tuple.__new__(cls, (modified, added, removed, deleted, unknown,
> > +                ignored, clean))
> > +
> > +    @property
> > +    def modified(self):
> > +        return self[0]
> 
> I think I would prefer to store the members by name (in slots?) and 
> avoid doing the array index lookup. That would make it more like an 
> object that behaved like a tuple than the opposite.

tuple can't have non-empty slot, so it needs __dict__ in order to store members
by name.

property access:

% python -m timeit -s 'class status(tuple):
    __slots__ = ()
    def __new__(cls, modified):
        return tuple.__new__(cls, (modified,))
    @property
    def modified(self):
        return self[0]
o = status([])' 'o.modified'
10000000 loops, best of 3: 0.153 usec per loop

named variable in __dict__:

% python -m timeit -s 'class status(tuple):
    def __new__(cls, modified):
        o = tuple.__new__(cls, (modified,))
        o.modified = modified
        return o
o = status([])' 'o.modified'
10000000 loops, best of 3: 0.0361 usec per loop

direct index access:

% python -m timeit -s 'class status(tuple):
    __slots__ = ()
    def __new__(cls, modified):
        return tuple.__new__(cls, (modified,))
    @property
    def modified(self):
        return self[0]
o = status([])' 'o[0]'
10000000 loops, best of 3: 0.0336 usec per loop

I don't know if this difference matters.

Regards,


More information about the Mercurial-devel mailing list