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 <[email protected]> > wrote: > One minor comment on this one: > > > On Thu, 2010-12-02 at 16:21 -0800, [email protected] 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 >
signature.asc
Description: This is a digitally signed message part
_______________________________________________ Mailing list: https://launchpad.net/~dulwich-users Post to : [email protected] Unsubscribe : https://launchpad.net/~dulwich-users More help : https://help.launchpad.net/ListHelp
