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

BaseNode.children is set as list, but get as tuple #66

Closed
lverweijen opened this issue Jul 10, 2023 · 3 comments · Fixed by #68
Closed

BaseNode.children is set as list, but get as tuple #66

lverweijen opened this issue Jul 10, 2023 · 3 comments · Fixed by #68
Labels
bug Something isn't working

Comments

@lverweijen
Copy link

It seems the children are set as a list, but are returned as a tuple.
I think it would make more sense to set/get it as the same type, probably a tuple.

It makes the following (no-op looking) code illegal:

node = Node("node")
node.children = node.children
# => TypeError: Children input should be list type, received input type <class 'tuple'>

It's risky too, because of the following:

root = Node("root")
mutable_children = list(map(Node, "abc"))

root.children = mutable_children
c = mutable_children.pop()

print(root.children)  # => (Node(/root/a, ), Node(/root/b, ))
print(c.parent)  # => Node(/root, )
raise ZeroDivisionError
@kayjan kayjan added the bug Something isn't working label Jul 11, 2023
@kayjan
Copy link
Owner

kayjan commented Jul 13, 2023

Thanks for spotting this issue! I have modified children to take in Iterable which can accept lists and tuples (and other iterable types) in v0.9.5 (unreleased yet but it is in the v0.9.5 branch). Do check it out and see if there are any other issues if not v0.9.5 will be rolled out soon 😃

@kayjan kayjan closed this as completed Jul 13, 2023
@Abdur-rahmaanJ
Copy link
Contributor

@kayjan Suggest to keep this and #67 and such issues open until the release has been made.

@kayjan kayjan reopened this Jul 13, 2023
@kayjan
Copy link
Owner

kayjan commented Jul 13, 2023

v0.9.5 is now live, do upgrade bigtree with pip install --upgrade bigtree to get the latest changes.

@kayjan kayjan closed this as completed Jul 13, 2023
@kayjan kayjan linked a pull request Oct 16, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants