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

New Tree.children_itr method #118

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

cowo78
Copy link

@cowo78 cowo78 commented Sep 12, 2019

In case creating a list like Tree.children() does is too heavy for efficiency reasons.

Giuseppe Corbelli added 2 commits September 12, 2019 10:23
case an iterator is preferrable over a list creation for efficiency
reasons.
nodes as its companion method Tree.children().
@caesar0301
Copy link
Owner

It seems as duplicated as method children which could convert into an iterator via iter(childrenI())

@cowo78
Copy link
Author

cowo78 commented Dec 2, 2019

Not really. Doing an iter(children()) would first create a list then iterate over it while children_itr() would just create the iterator. In the first case memory usage is O(N), in the second is O(1).

@@ -296,6 +296,13 @@ def children(self, nid):
"""
return [self[i] for i in self.is_branch(nid)]
Copy link

@crabhi crabhi Mar 20, 2020

Choose a reason for hiding this comment

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

High, just a quick note - this method could now be implemented equivalently as list(self.children_itr(nid)). That might address @caesar0301's comment re. redundancy.

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.

None yet

3 participants