← Back to team overview

dulwich-users team mailing list archive

Re: [PATCH 02/28] Add tree-diffing functionality in a 'diff' module.

 

On this topic, did we ever agree on name for this module, like tree_diff
instead of diff or something?

On Sun, Dec 12, 2010 at 15:13, Jelmer Vernooij <jelmer@xxxxxxxxx> wrote:

> One minor comment on this one:
>
> On Thu, 2010-12-02 at 16:21 -0800, dborowitz@xxxxxxxxxx wrote:
> > index 7c74751..c7f00c0 100644
> > --- a/dulwich/misc.py
> > +++ b/dulwich/misc.py
> > @@ -105,6 +105,7 @@ try:
> >      from collections import namedtuple
> >
> >      TreeEntryTuple = namedtuple('TreeEntryTuple', ['path', 'mode',
> 'sha'])
> > +    TreeChangeTuple = namedtuple('TreeChangeTuple', ['type', 'old',
> 'new'])
> >  except ImportError:
> >      # Provide manual implementations of namedtuples for Python <2.5.
> >      # If the class definitions change, be sure to keep these in sync by
> running
> > @@ -153,3 +154,43 @@ except ImportError:
> >              path = _property(_itemgetter(0))
> >              mode = _property(_itemgetter(1))
> >              sha = _property(_itemgetter(2))
> > +
> > +
> > +    class TreeChangeTuple(tuple):
> > +            'TreeChangeTuple(type, old, new)'
> > +
> > +            __slots__ = ()
> > +
> > +            _fields = ('type', 'old', 'new')
> > +
> > +            def __new__(_cls, type, old, new):
> > +                return _tuple.__new__(_cls, (type, old, new))
> > +
> > +            @classmethod
> > +            def _make(cls, iterable, new=tuple.__new__, len=len):
> > +                'Make a new TreeChangeTuple object from a sequence or
> iterable'
> > +                result = new(cls, iterable)
> > +                if len(result) != 3:
> > +                    raise TypeError('Expected 3 arguments, got %d' %
> len(result))
> > +                return result
> > +
> > +            def __repr__(self):
> > +                return 'TreeChangeTuple(type=%r, old=%r, new=%r)' % self
> > +
> > +            def _asdict(t):
> > +                'Return a new dict which maps field names to their
> values'
> > +                return {'type': t[0], 'old': t[1], 'new': t[2]}
> > +
> > +            def _replace(_self, **kwds):
> > +                'Return a new TreeChangeTuple object replacing specified
> fields with new values'
> > +                result = _self._make(map(kwds.pop, ('type', 'old',
> 'new'), _self))
> > +                if kwds:
> > +                    raise ValueError('Got unexpected field names: %r' %
> kwds.keys())
> > +                return result
> > +
> > +            def __getnewargs__(self):
> > +                return tuple(self)
> > +
> > +            type = _property(_itemgetter(0))
> > +            old = _property(_itemgetter(1))
> > +            new = _property(_itemgetter(2))
> I'd prefer to keep misc.py limited to compatibility code - on the latest
> python we should not be importing it at all. We already had
> TreeEntryTuple in that file so I realise this was already strictly no
> longer true, and it's hard to do given we don't have namedtuple on older
> Pythons.
>

Yeah, namedtuples are weird. There is a recipe to just implement namedtuple
in pure Python for <2.5, so I guess we could put that in misc.py and then
define namedtuples where we need them. I'm not sure about efficiency on 2.4.


> I'll merge this as is, but it'd be nice if we can come up with a way to
> avoid it. Perhaps we should also rename misc to _compat or something
> like that - to discourage users from importing from there.
>

+1 to s/misc/compat/.


> Cheers,
>
> Jelmer
>

Follow ups

References