New syntax for node children and paths (#587)

ChangeLog

@@ -5,6 +5,8 @@
 ### Changed
 - In the graphical interface, removed option to configure information set link drawing; information sets
   are always drawn and indicators are always drawn if an information set spans multiple levels.
+- In `pygambit`, indexing the children of a node by a string inteprets the string as an action label,
+  not a label of a child node.  In addition, indexing by an action object is now supported. (#587)
 
 ### Added
 - Tests for EFG Nash solvers -- `enumpoly_solve`, `lp_solve`, `lcp_solve` -- in behavior stratgegies

doc/pygambit.user.rst

@@ -48,7 +48,7 @@ We can then also add the Seller's move in the situation after the Buyer chooses
 
 .. ipython:: python
 
-   g.append_move(g.root.children[0], "Seller", ["Honor", "Abuse"])
+   g.append_move(g.root.children["Trust"], "Seller", ["Honor", "Abuse"])
 
 Now that we have the moves of the game defined, we add payoffs.  Payoffs are associated with
 an :py:class:`.Outcome`; each :py:class:`Outcome` has a vector of payoffs, one for each player,
@@ -57,20 +57,20 @@ Seller proving themselves trustworthy:
 
 .. ipython:: python
 
-   g.set_outcome(g.root.children[0].children[0], g.add_outcome([1, 1], label="Trustworthy"))
+   g.set_outcome(g.root.children["Trust"].children["Honor"], g.add_outcome([1, 1], label="Trustworthy"))
 
 Next, the outcome associated with the scenario where the Buyer trusts but the Seller does
 not return the trust:
 
 .. ipython:: python
 
-   g.set_outcome(g.root.children[0].children[1], g.add_outcome([-1, 2], label="Untrustworthy"))
+   g.set_outcome(g.root.children["Trust"].children["Abuse"], g.add_outcome([-1, 2], label="Untrustworthy"))
 
 And, finally the outcome associated with the Buyer opting out of the interaction:
 
 .. ipython:: python
 
-   g.set_outcome(g.root.children[1], g.add_outcome([0, 0], label="Opt-out"))
+   g.set_outcome(g.root.children["Not trust"], g.add_outcome([0, 0], label="Opt-out"))
 
 Nodes without an outcome attached are assumed to have payoffs of zero for all players.
 Therefore, adding the outcome to this latter terminal node is not strictly necessary in Gambit,
@@ -111,19 +111,19 @@ We can build this game using the following script::
         g.append_move(g.root, g.players.chance, ["King", "Queen"])
         for node in g.root.children:
             g.append_move(node, "Alice", ["Raise", "Fold"])
-        g.append_move(g.root.children[0].children[0], "Bob", ["Meet", "Pass"])
-        g.append_infoset(g.root.children[1].children[0],
-                         g.root.children[0].children[0].infoset)
+        g.append_move(g.root.children["King"].children["Raise"], "Bob", ["Meet", "Pass"])
+        g.append_infoset(g.root.children["Queen"].children["Raise"],
+                         g.root.children["King"].children["Raise"].infoset)
         alice_winsbig = g.add_outcome([2, -2], label="Alice wins big")
         alice_wins = g.add_outcome([1, -1], label="Alice wins")
         bob_winsbig = g.add_outcome([-2, 2], label="Bob wins big")
         bob_wins = g.add_outcome([-1, 1], label="Bob wins")
-        g.set_outcome(g.root.children[0].children[0].children[0], alice_winsbig)
-        g.set_outcome(g.root.children[0].children[0].children[1], alice_wins)
-        g.set_outcome(g.root.children[0].children[1], bob_wins)
-        g.set_outcome(g.root.children[1].children[0].children[0], bob_winsbig)
-        g.set_outcome(g.root.children[1].children[0].children[1], alice_wins)
-        g.set_outcome(g.root.children[1].children[1], bob_wins)
+        g.set_outcome(g.root.children["King"].children["Raise"].children["Meet"], alice_winsbig)
+        g.set_outcome(g.root.children["King"].children["Raise"].children["Pass"], alice_wins)
+        g.set_outcome(g.root.children["King"].children["Fold"], bob_wins)
+        g.set_outcome(g.root.children["Queen"].children["Raise"].children["Meet"], bob_winsbig)
+        g.set_outcome(g.root.children["Queen"].children["Raise"].children["Pass"], alice_wins)
+        g.set_outcome(g.root.children["Queen"].children["Fold"], bob_wins)
 
 All extensive games have a chance (or nature) player, accessible as
 ``.Game.players.chance``.  Moves belonging to the chance player can be added in the same
@@ -140,9 +140,9 @@ causes each of the newly-appended moves to be in new information sets.  In contr
 does not know Alice's card, and therefore cannot distinguish between the two nodes at which
 he has the decision.   This is implemented in the following lines::
 
-        g.append_move(g.root.children[0].children[0], "Bob", ["Meet", "Pass"])
-        g.append_infoset(g.root.children[1].children[0],
-                         g.root.children[0].children[0].infoset)
+        g.append_move(g.root.children["King"].children["Raise"], "Bob", ["Meet", "Pass"])
+        g.append_infoset(g.root.children["Queen"].children["Raise"],
+                         g.root.children["King"].children["Raise"].infoset)
 
 The call :py:meth:`.Game.append_infoset` adds a move at a terminal node as part of
 an existing information set (represented in ``pygambit`` as an :py:class:`.Infoset`).

src/games/game.h

@@ -82,6 +82,7 @@ class UndefinedException final : public std::runtime_error {
 class MismatchException : public std::runtime_error {
 public:
   MismatchException() : std::runtime_error("Operation between objects in different games") {}
+  explicit MismatchException(const std::string &s) : std::runtime_error(s) {}
   ~MismatchException() noexcept override = default;
 };
 
@@ -444,7 +445,7 @@ class GameNodeRep : public std::enable_shared_from_this<GameNodeRep> {
   GameNode GetChild(const GameAction &p_action)
   {
     if (p_action->GetInfoset().get() != m_infoset) {
-      throw MismatchException();
+      throw MismatchException("Action is from a different information set than node");
     }
     return m_children.at(p_action->GetNumber() - 1);
   }

src/pygambit/node.pxi

@@ -45,23 +45,37 @@ class NodeChildren:
         for child in self.parent.deref().GetChildren():
             yield Node.wrap(child)
 
-    def __getitem__(self, index: typing.Union[int, str]) -> Node:
-        if isinstance(index, str):
-            if not index.strip():
-                raise ValueError("Node label cannot be empty or all whitespace")
-            matches = [x for x in self if x.label == index.strip()]
-            if not matches:
-                raise KeyError(f"Node has no child with label '{index}'")
-            if len(matches) > 1:
-                raise ValueError(f"Node has multiple children with label '{index}'")
-            return matches[0]
-        if isinstance(index, int):
+    def __getitem__(self, action: typing.Union[int, str, Action]) -> Node:
+        """Returns the successor node which is reached after 'action' is played.
+
+        .. versionchanged: 16.5.0
+            Previously indexing by string searched the labels of the child nodes,
+            rather than referring to actions.  This implements the more natural
+            interpretation that strings refer to action labels.
+
+            Relatedly, the collection can now be indexed by an Action object.
+        """
+        if isinstance(action, str):
+            if not action.strip():
+                raise ValueError("Action label cannot be empty or all whitespace")
+            if self.parent.deref().GetInfoset() == cython.cast(c_GameInfoset, NULL):
+                raise ValueError(f"No action with label '{action}' at node")
+            for act in self.parent.deref().GetInfoset().deref().GetActions():
+                if act.deref().GetLabel().decode("ascii") == cython.cast(str, action):
+                    return Node.wrap(self.parent.deref().GetChild(act))
+            raise ValueError(f"No action with label '{action}' at node")
+        if isinstance(action, Action):
+            try:
+                return Node.wrap(self.parent.deref().GetChild(cython.cast(Action, action).action))
+            except IndexError:
+                raise ValueError(f"Action is from a different information set than node")
+        if isinstance(action, int):
             if self.parent.deref().GetInfoset() == cython.cast(c_GameInfoset, NULL):
                 raise IndexError("Index out of range")
             return Node.wrap(self.parent.deref().GetChild(
-                self.parent.deref().GetInfoset().deref().GetAction(index + 1)
+                self.parent.deref().GetInfoset().deref().GetAction(action + 1)
             ))
-        raise TypeError(f"Child index must be int or str, not {index.__class__.__name__}")
+        raise TypeError(f"Index must be int, str, or Action, not {action.__class__.__name__}")
 
 
 @cython.cclass

tests/games.py

@@ -146,28 +146,33 @@ def create_kuhn_poker_efg() -> gbt.Game:
     g = gbt.Game.new_tree(
         players=["Alice", "Bob"], title="Three-card poker (J, Q, K), two-player"
     )
+    cards = ["J", "Q", "K"]
     deals = ["JQ", "JK", "QJ", "QK", "KJ", "KQ"]
+
+    def deals_by_infoset(player, card):
+        player_idx = 0 if player == "Alice" else 1
+        return [d for d in deals if d[player_idx] == card]
+
     g.append_move(g.root, g.players.chance, deals)
     g.set_chance_probs(g.root.infoset, [gbt.Rational(1, 6)]*6)
-    # group the children of the root (indices of `deals`) by each player's dealt card
-    alice_grouping = [[0, 1], [2, 3], [4, 5]]  # J, Q, K
-    bob_grouping = [[0, 5], [1, 3], [2, 4]]  # Q, K, J
-
-    # Alice's first move
-    for ij in alice_grouping:
-        term_nodes = [g.root.children[k] for k in ij]
+    for alice_card in cards:
+        # Alice's first move
+        term_nodes = [g.root.children[d] for d in deals_by_infoset("Alice", alice_card)]
         g.append_move(term_nodes, "Alice", ["Check", "Bet"])
-    # Bob's move after Alice checks
-    for ij in bob_grouping:
-        term_nodes = [g.root.children[k].children[0] for k in ij]
+    for bob_card in cards:
+        # Bob's move after Alice checks
+        term_nodes = [g.root.children[d].children["Check"]
+                      for d in deals_by_infoset("Bob", bob_card)]
         g.append_move(term_nodes, "Bob", ["Check", "Bet"])
-    # Alice's move if Bob's second action is bet
-    for ij in alice_grouping:
-        term_nodes = [g.root.children[k].children[0].children[1] for k in ij]
+    for alice_card in cards:
+        # Alice's move if Bob's second action is bet
+        term_nodes = [g.root.children[d].children["Check"].children["Bet"]
+                      for d in deals_by_infoset("Alice", alice_card)]
         g.append_move(term_nodes, "Alice", ["Fold", "Call"])
-    # Bob's move after Alice bets initially
-    for ij in bob_grouping:
-        term_nodes = [g.root.children[k].children[1] for k in ij]
+    for bob_card in cards:
+        # Bob's move after Alice bets initially
+        term_nodes = [g.root.children[d].children["Bet"]
+                      for d in deals_by_infoset("Bob", bob_card)]
         g.append_move(term_nodes, "Bob", ["Fold", "Call"])
 
     def calculate_payoffs(term_node):

tests/test_node.py

@@ -790,6 +790,29 @@ def test_node_plays():
     assert set(test_node.plays) == expected_set_of_plays
 
 
+def test_node_children_action_label():
+    game = games.read_from_file("poker.efg")
+    assert game.root.children["Red"] == game.root.children[0]
+    assert game.root.children["Black"].children["Fold"] == game.root.children[1].children[1]
+
+
+def test_node_children_action():
+    game = games.read_from_file("poker.efg")
+    assert game.root.children[game.root.infoset.actions["Red"]] == game.root.children[0]
+
+
+def test_node_children_nonexistent_action():
+    game = games.read_from_file("poker.efg")
+    with pytest.raises(ValueError):
+        _ = game.root.children["Green"]
+
+
+def test_node_children_other_infoset_action():
+    game = games.read_from_file("poker.efg")
+    with pytest.raises(ValueError):
+        _ = game.root.children[game.root.children[0].infoset.actions["Raise"]]
+
+
 @pytest.mark.parametrize(
     "game_obj",
     [