Repository navigation
Walk NestedTree nesting levels with loops instead of recursion - #106
Merged
Merged
Conversation
NestedTree.__getitem__ (through _lookup_entry) and parent_position (through _parent_position) called themselves once per nesting level, so about a thousand Tree objects nested inside each other raised RecursionError. Both are now loops; parent_position looks up the trees along the position once instead of once per level. Closes #101. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #101.
NestedTree.__getitem__(through_lookup_entry) andparent_position(through_parent_position) called themselves once per nesting level, that is perTreeheld as a node of anotherTree. About a thousand nestedTreeobjects raisedRecursionError. These were the last recursive walks innested.py.Both are now loops.
__getitem__steps into the nested trees and, as before, stops at an entry that is not aTree.parent_positionlooks up the trees along the position once with_treesand works outwards from the innermost level until one has a parent, as the sibling moves already do; the old code looked the trees up again at every level. The two private helpers are deleted. No public behaviour changes.test_deep_nestingnests 1100SimpleTreeobjects and checks the walk in both directions, parents, siblings and depth. It checks against expected values directly, because theflattenhelper of the invariant tests itself recurses per nesting level. Without the fix it raisesRecursionError, and it still fails if either of the two functions is restored on its own.Known limit, outside this change: if every one of about a thousand nesting levels is decorated (for example each wrapped in
ArrowTree), rendering still raisesRecursionErrorinside urwid, because the decorated line becomes as many nestedurwid.Columns. Navigation andget_decoratedwork on such a tree.Review findings and resolution
review-correctness
_lookup_entrywas: fixed.Columns: left, as it lies outside urwidtrees' navigation and this issue; recorded above as a known limit.ArrowTreeandCollapsibleArrowTreesubtrees, with and withoutinterpret_covered; deep chains, 10000 children and 10000 subtrees run as fast as onmain.review-design
_treesand follows the inside-out loop used elsewhere in the module; the public API is unchanged.🤖 Generated with Claude Code