Repository: Textualize/textual
Pull request: Add read-only access to the children of a `TreeNode`
Adds a generic `ImmutableSequence` wrapper class (name and location totally up for debate, although the name of the class got the nod in a Slack chat so I suspect that's okay) and then goes on to use that to wrap the internal child list of a `TreeNode`, making it available in a non-destructive way.

See #1398.

Problem statement:
[Tree] Make TreeNode an immutable Sequence to allow for walking the tree
Especially to allow some initial "just so" setup of a `Tree`, but also to make it easy to write code to walk a `Tree`, it would be useful to have public access to the `children` of a `TreeNode`.

Hints from issue discussion:
👍 I just needed this for writing code that "focuses" the tree on a specific node, collapsing all nodes other than one specific node. In my use-case I get only the *names* of the nodes (labels), not the nodes themselves, along them path; so have to traverse children.
Making the list public invites a few problems. A dev might expect to be able to extend, remove etc and the tree to update accordingly.

I think we should provide for the use-case of iterating children, and walking a sub-tree without exposing the list.
I'm thinking perhaps we should make TreeNode a Sequence, to allow for iterating. This would allow for walking a tree while not exposing internals.

Commit message: Add read-only access to the children of a TreeNode

See #1398.

Patch to review:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 876edeb4b6..f31ca177a0 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [0.10.0] - Unreleased
 
+### Added
+
+- Added read-only public access to the children of a `TreeNode` via `TreeNode.children` https://github.com/Textualize/textual/issues/1398
+
 ### Changed
 
 - `MouseScrollUp` and `MouseScrollDown` now inherit from `MouseEvent` and have attached modifier keys. https://github.com/Textualize/textual/pull/1458
@@ -29,8 +33,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 - Widget.render_line now returns a Strip
 - Fix for slow updates on Windows
-- Bumped Rich dependency 
-  
+- Bumped Rich dependency
+
 ## [0.8.2] - 2022-12-28
 
 ### Fixed
diff --git a/src/textual/_collections.py b/src/textual/_collections.py
new file mode 100644
index 0000000000..09b8af206d
--- /dev/null
+++ b/src/textual/_collections.py
@@ -0,0 +1,65 @@
+"""Provides collection-based utility code."""
+
+from __future__ import annotations
+from typing import Generic, TypeVar, Iterator, overload, Iterable
+
+T = TypeVar("T")
+
+
+class ImmutableSequence(Generic[T]):
+    """Class to wrap a sequence of some sort, but not allow modification."""
+
+    def __init__(self, wrap: Iterable[T]) -> None:
+        """Initialise the immutable sequence.
+
+        Args:
+            wrap (Iterable[T]): The iterable value being wrapped.
+        """
+        self._list = list(wrap)
+
+    @overload
+    def __getitem__(self, index: int) -> T:
+        ...
+
+    @overload
+    def __getitem__(self, index: slice) -> ImmutableSequence[T]:
+        ...
+
+    def __getitem__(self, index: int | slice) -> T | ImmutableSequence[T]:
+        return (
+            self._list[index]
+            if isinstance(index, int)
+            else ImmutableSequence[T](self._list[index])
+        )
+
+    def __iter__(self) -> Iterator[T]:
+        return iter(self._list)
+
+    def __len__(self) -> int:
+        return len(self._list)
+
+    def __length_hint__(self) -> int:
+        return len(self)
+
+    def __bool__(self) -> bool:
+        return bool(len(self))
+
+    def __contains__(self, item: T) -> bool:
+        return item in self._list
+
+    def index(self, item: T) -> int:
+        """Return the index of the given item.
+
+        Args:
+            item (T): The item to find in the sequence.
+
+        Returns:
+            int: The index of the item in the sequence.
+
+        Raises:
+            ValueError: If the item is not in the sequence.
+        """
+        return self._list.index(item)
+
+    def __reversed__(self) -> Iterator[T]:
+        return reversed(self._list)
diff --git a/src/textual/widgets/_tree.py b/src/textual/widgets/_tree.py
index e23385502c..4d4d251a95 100644
--- a/src/textual/widgets/_tree.py
+++ b/src/textual/widgets/_tree.py
@@ -14,6 +14,7 @@
 from .._segment_tools import line_crop, line_pad
 from .._types import MessageTarget
 from .._typing import TypeAlias
+from .._collections import ImmutableSequence
 from ..binding import Binding
 from ..geometry import Region, Size, clamp
 from ..message import Message
@@ -53,6 +54,10 @@ def _get_guide_width(self, guide_depth: int, show_root: bool) -> int:
         return guides
 
 
+class TreeNodes(ImmutableSequence["TreeNode[TreeDataType]"]):
+    """An immutable collection of `TreeNode`."""
+
+
 @rich.repr.auto
 class TreeNode(Generic[TreeDataType]):
     """An object that represents a "node" in a tree control."""
@@ -74,7 +79,7 @@ def __init__(
         self._label = label
         self.data = data
         self._expanded = expanded
-        self._children: list[TreeNode] = []
+        self._children: list[TreeNode[TreeDataType]] = []
 
         self._hover_ = False
         self._selected_ = False
@@ -91,6 +96,11 @@ def _reset(self) -> None:
         self._selected_ = False
         self._updates += 1
 
+    @property
+    def children(self) -> TreeNodes[TreeDataType]:
+        """TreeNodes[TreeDataType]: The child nodes of a TreeNode."""
+        return TreeNodes(self._children)
+
     @property
     def line(self) -> int:
         """int: Get the line number for this node, or -1 if it is not displayed."""
diff --git a/tests/test_collections.py b/tests/test_collections.py
new file mode 100644
index 0000000000..cd4d0f71e6
--- /dev/null
+++ b/tests/test_collections.py
@@ -0,0 +1,81 @@
+import pytest
+
+from typing import Iterable
+from textual._collections import ImmutableSequence
+
+def wrap(source: Iterable[int]) -> ImmutableSequence[int]:
+    """Wrap an itertable of integers inside an immutable sequence."""
+    return ImmutableSequence[int](source)
+
+
+def test_empty_immutable_sequence() -> None:
+    """An empty immutable sequence should act as anticipated."""
+    assert len(wrap([])) == 0
+    assert bool(wrap([])) is False
+    assert list(wrap([])) == []
+
+
+def test_non_empty_immutable_sequence() -> None:
+    """A non-empty immutable sequence should act as anticipated."""
+    assert len(wrap([0])) == 1
+    assert bool(wrap([0])) is True
+    assert list(wrap([0])) == [0]
+
+
+def test_immutable_sequence_from_empty_iter() -> None:
+    """An immutable sequence around an empty iterator should act as anticipated."""
+    assert len(wrap([])) == 0
+    assert bool(wrap([])) is False
+    assert list(wrap(iter([]))) == []
+
+
+def test_immutable_sequence_from_non_empty_iter() -> None:
+    """An immutable sequence around a non-empty iterator should act as anticipated."""
+    assert len(wrap(range(23))) == 23
+    assert bool(wrap(range(23))) is True
+    assert list(wrap(range(23))) == list(range(23))
+
+
+def test_no_assign_to_immutable_sequence() -> None:
+    """It should not be possible to assign into an immutable sequence."""
+    tester = wrap([1,2,3,4,5])
+    with pytest.raises(TypeError):
+        tester[0] = 23
+    with pytest.raises(TypeError):
+        tester[0:3] = 23
+
+
+def test_no_del_from_iummutable_sequence() -> None:
+    """It should not be possible delete an item from an immutable sequence."""
+    tester = wrap([1,2,3,4,5])
+    with pytest.raises(TypeError):
+        del tester[0]
+
+
+def test_get_item_from_immutable_sequence() -> None:
+    """It should be possible to get an item from an immutable sequence."""
+    assert wrap(range(10))[0] == 0
+    assert wrap(range(10))[-1] == 9
+
+def test_get_slice_from_immutable_sequence() -> None:
+    """It should be possible to get a slice from an immutable sequence."""
+    assert list(wrap(range(10))[0:2]) == [0,1]
+    assert list(wrap(range(10))[0:-1]) == [0,1,2,3,4,5,6,7,8]
+
+
+def test_immutable_sequence_contains() -> None:
+    """It should be possible to see if an immutable sequence contains a value."""
+    tester = wrap([1,2,3,4,5])
+    assert 1 in tester
+    assert 11 not in tester
+
+
+def test_immutable_sequence_index() -> None:
+    tester = wrap([1,2,3,4,5])
+    assert tester.index(1) == 0
+    with pytest.raises(ValueError):
+        _ = tester.index(11)
+
+
+def test_reverse_immutable_sequence() -> None:
+    assert list(reversed(wrap([1,2]))) == [2,1]
diff --git a/tests/tree/test_tree_node_children.py b/tests/tree/test_tree_node_children.py
new file mode 100644
index 0000000000..22df664dba
--- /dev/null
+++ b/tests/tree/test_tree_node_children.py
@@ -0,0 +1,32 @@
+import pytest
+from textual.widgets import Tree, TreeNode
+
+def label_of(node: TreeNode[None]):
+    """Get the label of a node.
+
+    TODO: This is just a helper function to reduce the number of type
+    errors, which can and will be remove once this code is merged with a
+    version of main that also has the TreeNode.label PR merged.
+    """
+    return str(node._label)
+
+
+def test_tree_node_children() -> None:
+    """A node's children property should act like an immutable list."""
+    CHILDREN=23
+    tree = Tree[None]("Root")
+    for child in range(CHILDREN):
+        tree.root.add(str(child))
+    assert len(tree.root.children)==CHILDREN
+    for child in range(CHILDREN):
+        assert label_of(tree.root.children[child]) == str(child)
+    assert label_of(tree.root.children[0]) == "0"
+    assert label_of(tree.root.children[-1]) == str(CHILDREN-1)
+    assert [label_of(node) for node in tree.root.children] == [str(n) for n in range(CHILDREN)]
+    assert [label_of(node) for node in tree.root.children[:2]] == [str(n) for n in range(2)]
+    with pytest.raises(TypeError):
+        tree.root.children[0] = tree.root.children[1]
+    with pytest.raises(TypeError):
+        del tree.root.children[0]
+    with pytest.raises(TypeError):
+        del tree.root.children[0:2]


Code review comments:
diff_hunk: @@ -0,0 +1,65 @@
+"""Provides collection-based utility code."""
+
+from __future__ import annotations
+from typing import Generic, TypeVar, Iterator, overload, Iterable
+
+T = TypeVar("T")
+
+
+class ImmutableSequence(Generic[T]):
+    """Class to wrap a sequence of some sort, but not allow modification."""
+
+    def __init__(self, wrap: Iterable[T]) -> None:
+        """Initialise the immutable sequence.
+
+        Args:
+            wrap (Iterable[T]): The iterable value being wrapped.
+        """
+        self._list = list(wrap)
+
+    @overload
+    def __getitem__(self, index: int) -> T:
+        ...
+
+    @overload
+    def __getitem__(self, index: slice) -> ImmutableSequence[T]:
+        ...
+
+    def __getitem__(self, index: int | slice) -> T | ImmutableSequence[T]:
+        return (
+            self._list[index]
+            if isinstance(index, int)
+            else ImmutableSequence[T](self._list[index])
+        )
+
+    def __iter__(self) -> Iterator[T]:
+        return iter(self._list)
+
+    def __len__(self) -> int:
+        return len(self._list)
+
+    def __length_hint__(self) -> int:
+        return len(self)
+
+    def __bool__(self) -> bool:
+        return bool(len(self))
path: src/textual/_collections.py
text: @user1:
wondering out loud: doesn't `Sequence` already define `__bool__`; and if so -- why not just `return bool(self._list)`?

@author:
Yeah, good call, probably makes sense to whittle it down to that (mostly the above is a handover from a slightly earlier version).

diff_hunk: @@ -0,0 +1,65 @@
+"""Provides collection-based utility code."""
+
+from __future__ import annotations
+from typing import Generic, TypeVar, Iterator, overload, Iterable
+
+T = TypeVar("T")
+
+
+class ImmutableSequence(Generic[T]):
+    """Class to wrap a sequence of some sort, but not allow modification."""
+
+    def __init__(self, wrap: Iterable[T]) -> None:
+        """Initialise the immutable sequence.
+
+        Args:
+            wrap (Iterable[T]): The iterable value being wrapped.
+        """
+        self._list = list(wrap)
path: src/textual/_collections.py
text: @user1:
wouldn't it be better to just hold a reference to the original sequence? Why copy it? We're not allowing modification anyway.

@user1:
(and, not to retract from the above: if copying, why not make it tuple() while we're at it, to REALLY disallow modification?)

@author:
> wouldn't it be better to just hold a reference to the original sequence? Why copy it? We're not allowing modification anyway.

The request for this was, in part, that it be able to take any reasonable iterable while allowing it to be used list-like.

@user1:
I'll give it one more shot, then:

1. I was thinking more something like `KeysView` and `ItemsView` from Python's stdlib. They are "aware of perf tradeoffs", so to speak. So here we would have `SequenceView`, and use it where appropriate. There can be also `ImmutableSequence`, but it doesn't mean it has to be used in the particular case of TreeNode's children.
2. How about adding a check in ImmutableSequence's `__init__()`: if `wrap` is an instance of `Sequence` then just store a reference; other convert to tuple.

Apologies for "insisting", I promise this is the last of them, and hope you find the above useful.

@author:
Nah, appreciate it. Of course unlike those views I want this to be a subscriptable object. But yes, you're right, makes more sense to only convert when absolutely needed.