Skip to content

Tree algorithms - #1

Open
akdel wants to merge 42 commits into
holmrenser:masterfrom
akdel:master
Open

Tree algorithms#1
akdel wants to merge 42 commits into
holmrenser:masterfrom
akdel:master

Conversation

@akdel

@akdel akdel commented Aug 4, 2020

Copy link
Copy Markdown

Nj-tree with bootstrapping

@holmrenser holmrenser left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some comments on the clade and clades classes.

Comment on lines +13 to +42
@dataclass
class Clade:
internode: Tree
clade: Set[str]
depth: int

@classmethod
def from_internode(cls, internode: Tree, banned: [None, Set[str]] = None) -> 'Clade':
"""
Creates Clade from a given internode.
:param internode: A node which is not a leaf.
:param banned: banned node names.
:return: Clade instance
"""
assert internode.children is not None # makes sure that it's not a leaf
if banned is None:
banned = set()
clade: Set[str] = {x.name for x in internode.leaves if x.name not in banned} # removes any banned leaves. this is used in bootstrapping.
return cls(internode, clade, internode.depth)

def compare_to(self, other: 'Clade') -> bool:
"""
Compares the clade leaves to another.
:param other: Clade instance to be compared
:return: returns True if clades are equal.
"""
if self.clade == other.clade:
return True
else:
return False

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my perspective a clade is a subtree. What is the use of making this a separate class?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, you're right. I wasn't sure where I was going in the beginning, that's why I didn't want to touch the main class. I'll add them as a Tree method

Comment thread picea/algorithms/bootstrap.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants