I think the logic is flawed actually. What happens if multiple ancestors
have None as the name?

I believe it should be possible to use accumulate_to_ancestor here, which
would avoid the while loop. It won't be algorithmically optimal
(same/similar runtime to this method), but I don't think it will be
prohibitive in practice (ie, merge first, bench later)
On May 13, 2014 7:25 PM, "Greg Caporaso" <notifications@github.com> wrote:

> In skbio/core/tree.py:
>
> > +            all_tips = self.subset()
> > +            if not set(tip_subset).issubset(all_tips):
> > +                raise ValueError('tip_subset contains ids that arent tip '
> > +                                 'names.')
> > +
> > +            lca = self.lowest_common_ancestor(tip_subset)
> > +            ancestors = {}
> > +            for tip in tip_subset:
> > +                curr = self.find(tip)
> > +                while curr is not lca:
> > +                    ancestors[curr.name] = curr.length if curr.length is not \
> > +                        None else 0.0
> > +                    curr = curr.parent
> > +            return sum(ancestors.values())
> > +
> > +        else:
>
> @wasade <https://github.com/wasade>, do you think this performance test
> is essential for merge, or could we add an issue? in general we've been
> focusing on the functionality/documentation for this initial release, and
> performance optimization as necessary. If you think this is essential, I
> can help out to get it done. Otherwise, could you merge?
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/biocore/scikit-bio/pull/309/files#r12617515>
> .
>