New public Graph.revision_relation method

[Martin von Gagern]

John Arbash Meinel wrote:
> The internals are pretty much 100% different. KnownGraph expects to be
> passed the full parent_map of ancestry, while Graph expects to be passed
> a 'parents_provider' which it can then query for more ancestry.

I know, but as long as the interface is the same, or at least shares
enough common methods, more common methods could build on these.

> The internal layout of structures, etc, are also completely different.
> So while if you implement your functionality strictly in terms of
> '.heads()' then it could use either implementation, but if you implement
> something that depends on internal state, then it won't work on the
> other implementation.

heads is enough for me.

> It comes down to what can you expect to be part of the abstract base /
> interface.

I guess only heads for now, but I'd expect more to follow in the long
run, even if I couldn't name them.

> I'd be a bit cautious about inheriting from an abstract base in the
> Pyrex code, (for performance reasons), but it has been done before. the
> _annotator_pyx extension inherits from the pure-python version, and just
> overrides a couple key functions with pyrex optimized functions.

I have a similar concern, which is why I suggested multiple inheritance:
have the Pyrex implementation as one base class and the abstract graph
as the other base class. That way I could be sure that pyrex can use
optimized access to class members as well, not only to functions, but
still high-level Python methods could be shared between implementations.

As an alternative, one could also have a wrapper class that has a Pyrex
KnownGraph as a member and delegates all function calls to that. Would
avoid the multiple inheritance, and thereby make sure that there is no
penalty caused by inheritance between Pyrex and non-Pyrex classes. Maybe
someone who knows a bit more about Pyrex would know which way to choose?

> A simpler fashion would be to implement abstract functionality in a
> different class, and pass in either a KnownGraph instance or a Graph
> instance.

Then we might as well simply rename Branch._revision_relations, as that
already is a method in a different class taking a graph as an argument
and calculating the intended result. But I'd rather see an algorithm
implemented as a method of the object it's dealing with.

Martin

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 262 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20091210/3023081e/attachment.pgp