Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tree edit operations (NNI,SPR,TBR) #8

Open
sriram98v opened this issue Oct 24, 2023 · 4 comments · Fixed by #9
Open

Tree edit operations (NNI,SPR,TBR) #8

sriram98v opened this issue Oct 24, 2023 · 4 comments · Fixed by #9
Assignees

Comments

@sriram98v
Copy link
Owner

No description provided.

@sriram98v
Copy link
Owner Author

@swagle8987 SPR is implemented for rooted tree. Can NNI and TBR be implemented for and unrooted tree? If so, could you get that part?

@sriram98v sriram98v linked a pull request Oct 25, 2023 that will close this issue
@swagle8987
Copy link
Collaborator

Hey @sriram98v for the subtree and prune operations, shouldn't they consume the value of the tree sent in instead of borrowing a mutable reference to self? It would allow us to be more explicit in the fact that these operations are changing the structure of the tree and that the tree after the operation is not the same. If we aren't mutating the original tree then we could just pass a reference anyways instead of sending a mutable reference.

@sriram98v sriram98v reopened this Oct 28, 2023
@sriram98v
Copy link
Owner Author

sriram98v commented Oct 28, 2023

@swagle8987 The idea for subtree is to return a copy of the subtree starting at the input node without removing it from self. prune removes the subtree from self (hence using a &mut self) and returns the subtree, without consuming self. Graft consumes the input tree and grafts it to self (mutates self, hence &mut self)

@swagle8987
Copy link
Collaborator

@sriram98v yes but I think it's better if prune and regraft take ownership of the value rather than borrowing a reference. The subtree method can be a "view" method which returns the subtree at the node(although this wouldnt keep track of the changes made within the subtree of the original tree) but the prune and regraft method fundamentally change the tree and hence explicitly for the end user, they consume the original tree.
This also avoids bugs since now users don't have to compulsorily keep the tree variable as mutable and can instead create a mutable clone for prune and regraft.

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 a pull request may close this issue.

2 participants