← Back to team overview

dulwich-users team mailing list archive

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

 

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?

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 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
> 

Attachment: signature.asc
Description: This is a digitally signed message part


Follow ups

References