← Back to team overview

dulwich-users team mailing list archive

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

 

On Mon, Dec 13, 2010 at 04:27, Jelmer Vernooij <jelmer@xxxxxxxxx> wrote:

> On Sun, 2010-12-12 at 18:31 -0800, Dave Borowitz wrote:
> > On this topic, did we ever agree on name for this module, like
> > tree_diff instead of diff or something?
> No, I don't think we did. My preference still is to use something other
> than "dulwich.diff". If C git uses diff-tree for this sort of thing then
> I think using something similar ("diff_tree"?) would be reasonable, but
> I have no particular preference. Would diff_tree be ok with you?
>

diff_tree it is.


> Would you like to send a new patchset or should we just do the rename
> after applying the current patchset ? Either works for me.
>

I'll send a new patchset, rebased on your current master.


> I had a look over the rest of your changes this weekend and would be
> happy to merge them once we agree on the name.
>
> Cheers,
>
> Jelmer
>
> > 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
> >
>
>

References