diff --git a/ChangeLog b/ChangeLog index bb08023c3..932528071 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,11 @@ # Changelog +## [16.2.2] - unreleased + +### Fixed +- `Game.copy_tree` and `Game.move_tree` implementations reversed the roles of the + `src` and `dest` nodes (#499) + ## [16.2.1] - 2025-01-06 ### Fixed @@ -10,7 +16,6 @@ - Attempting to call the default constructor on Game objects (rather than one of the factory functions) now raises a more informative exception (#463) - ## [16.2.0] - 2024-04-05 ### Fixed diff --git a/src/pygambit/game.pxi b/src/pygambit/game.pxi index 2332d0e8a..4f4c7fc74 100644 --- a/src/pygambit/game.pxi +++ b/src/pygambit/game.pxi @@ -1362,7 +1362,16 @@ class Game: resolved_node.node.deref().InsertMove(resolved_infoset.infoset) def copy_tree(self, src: typing.Union[Node, str], dest: typing.Union[Node, str]) -> None: - """Copy the subtree rooted at 'src' to 'dest'. + """Copy the subtree rooted at the node `src` to the node `dest`. + + Each node in the subtree copied to follow `dest` is placed in the same information set + as the corresponding node in the original subtree under `src`. + + It is permitted for `dest` to be a descendant of `src`. + The operation uses the subtree rooted at `src` as it is at the time the function is called, + so no infinite recursion is triggered. + + The outcome associated with `dest` is not changed by this operation. Parameters ---------- @@ -1382,7 +1391,7 @@ class Game: resolved_dest = cython.cast(Node, self._resolve_node(dest, "copy_tree", "dest")) if not resolved_dest.is_terminal: raise UndefinedOperationError("copy_tree(): `dest` must be a terminal node.") - resolved_src.node.deref().CopyTree(resolved_dest.node) + resolved_dest.node.deref().CopyTree(resolved_src.node) def move_tree(self, src: typing.Union[Node, str], dest: typing.Union[Node, str]) -> None: """Move the subtree rooted at 'src' to 'dest'. @@ -1407,7 +1416,7 @@ class Game: raise UndefinedOperationError("move_tree(): `dest` must be a terminal node.") if resolved_dest.is_successor_of(resolved_src): raise UndefinedOperationError("move_tree(): `dest` cannot be a successor of `src`.") - resolved_src.node.deref().MoveTree(resolved_dest.node) + resolved_dest.node.deref().MoveTree(resolved_src.node) def delete_parent(self, node: typing.Union[Node, str]) -> None: """Delete the parent node of `node`. `node` replaces its parent in the tree. All other diff --git a/tests/test_node.py b/tests/test_node.py index 9b4c5eaf1..2a08b1817 100644 --- a/tests/test_node.py +++ b/tests/test_node.py @@ -1,3 +1,4 @@ +import typing import unittest import pygambit @@ -5,6 +6,31 @@ from . import games +# an auxiliary function used in `copy_tree` tests +def subtrees_equal( + n1: pygambit.Node, + n2: pygambit.Node, + recursion_stop_node: typing.Union[pygambit.Node, None] = None + ) -> bool: + if n1 == recursion_stop_node: + return n2.is_terminal + if n1.is_terminal and n2.is_terminal: + return n1.outcome == n2.outcome + if n1.is_terminal is not n2.is_terminal: + return False + # now, both n1 and n2 are non-terminal + # check that they are in the same infosets + if n1.infoset != n2.infoset: + return False + # check that they have the same number of children + if len(n1.children) != len(n2.children): + return False + + return all( + subtrees_equal(c1, c2, recursion_stop_node) for (c1, c2) in zip(n1.children, n2.children) + ) + + class TestGambitNode(unittest.TestCase): def setUp(self): self.game = pygambit.Game.new_tree() @@ -205,6 +231,26 @@ def test_node_copy_across_games(self): self.assertRaises(pygambit.MismatchError, self.game.copy_tree, self.extensive_game.root, self.game.root) + def test_copy_tree_onto_nondescendant_terminal_node(self): + + g = games.read_from_file("e01.efg") + src_node = g.nodes()[3] # path=[1, 0] + dest_node = g.nodes()[2] # path=[0, 0] + + g.copy_tree(src_node, dest_node) + + assert subtrees_equal(src_node, dest_node) + + def test_copy_tree_onto_descendant_terminal_node(self): + + g = games.read_from_file("e01.efg") + src_node = g.nodes()[1] # path=[0] + dest_node = g.nodes()[4] # path=[0, 1, 0] + + g.copy_tree(src_node, dest_node) + + assert subtrees_equal(src_node, dest_node, dest_node) + def test_node_move_nonterminal(self): """Test on moving to a nonterminal node.""" self.assertRaises(pygambit.UndefinedOperationError, self.extensive_game.move_tree,