diff --git a/ChangeLog b/ChangeLog index d778101df2..15c785f8c0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -7,6 +7,7 @@ - `gambit-ipa` now supports verbose output via the `-V` flag, reporting on intermediate progress. (#501) - Added `-R` option to `gambit-liap`, `gambit-simpdiv`, `gambit-ipa`, and `gambit-gnm` to reproducibly seed random starting point generation. (#448) +- Added `Game.relabel_players`, which simultaneously reassigns the labels of the game's players. (#1058) ### Fixed - `Game.add_player`, on a game in strategic form, no longer leaves the new player's sole @@ -36,6 +37,8 @@ were subsumed by `SetStrategies`, and by this point were used only in support of `pygambit`'s `add_strategy`/`delete_strategy` and the GUI's strategy add/delete controls, which now build the equivalent `SetStrategies` call directly. (#1056) + - Assigning to `Player.label` has been removed; use `Game.relabel_players`, which enforces + nonempty, unique labels. (#1058) ## [17.0.0-alpha.1] - 2026-08-13 diff --git a/doc/pygambit.api.rst b/doc/pygambit.api.rst index 62074e5486..3d71191873 100644 --- a/doc/pygambit.api.rst +++ b/doc/pygambit.api.rst @@ -85,6 +85,7 @@ Transforming game components :toctree: api/ Game.add_player + Game.relabel_players Game.add_outcome Game.delete_outcome Game.set_outcome diff --git a/doc/tutorials/01_quickstart.ipynb b/doc/tutorials/01_quickstart.ipynb index c1ad89a233..b1fba454f6 100644 --- a/doc/tutorials/01_quickstart.ipynb +++ b/doc/tutorials/01_quickstart.ipynb @@ -76,7 +76,14 @@ "id": "9d8203e8", "metadata": {}, "outputs": [], - "source": "tom, jerry = g.players\ntom.label = \"Tom\"\njerry.label = \"Jerry\"\n\nfor player in g.players:\n cooperate, defect = player.strategies\n g.relabel_strategies(player, {cooperate.label: \"Cooperate\", defect.label: \"Defect\"})" + "source": [ + "tom, jerry = g.players\n", + "g.relabel_players({tom.label: \"Tom\", jerry.label: \"Jerry\"})\n", + "\n", + "for player in g.players:\n", + " cooperate, defect = player.strategies\n", + " g.relabel_strategies(player, {cooperate.label: \"Cooperate\", defect.label: \"Defect\"})" + ] }, { "cell_type": "markdown", diff --git a/src/games/file.cc b/src/games/file.cc index 750ee23624..b0d48c61c4 100644 --- a/src/games/file.cc +++ b/src/games/file.cc @@ -375,24 +375,6 @@ template void NormalizeLabelStrings(Container &p_labels) [](std::string &s, const std::string &v) { s = v; }); } -template -void RelabelWithoutCollision(Container &&p_container, const std::vector &p_labels) -{ - const std::set forbidden(p_labels.begin(), p_labels.end()); - size_t scratchIndex = 0; - for (auto &&element : p_container) { - std::string candidate; - do { - candidate = "_gambit_reader_scratch_" + std::to_string(scratchIndex++); - } while (forbidden.contains(candidate)); - element->SetLabel(candidate); - } - size_t index = 0; - for (auto &&element : p_container) { - element->SetLabel(p_labels[index++]); - } -} - void ReadPlayers(GameFileLexer &p_state, TableFileGame &p_data) { p_state.ExpectNextToken(TOKEN_LBRACE, "'{'"); @@ -538,7 +520,12 @@ Game BuildNfg(GameFileLexer &p_parser, TableFileGame &p_data) playerLabels.push_back(p_data.GetPlayer(player->GetNumber())); } NormalizeLabelStrings(playerLabels); - RelabelWithoutCollision(nfg->GetPlayers(), playerLabels); + std::map playerRelabels; + size_t playerIndex = 0; + for (auto player : nfg->GetPlayers()) { + playerRelabels[player->GetLabel()] = playerLabels[playerIndex++]; + } + nfg->RelabelPlayers(playerRelabels); for (auto player : nfg->GetPlayers()) { std::vector strategyLabels; @@ -907,7 +894,10 @@ void NormalizeGameLabels(const Game &p_game) { const auto get_label = [](const auto &e) { return e->GetLabel(); }; const auto set_label = [](const auto &e, const std::string &s) { e->SetLabel(s); }; - NormalizeLabels(p_game->GetPlayers(), get_label, set_label); + const auto relabel_player = [&p_game](const auto &e, const std::string &s) { + p_game->RelabelPlayers({{e->GetLabel(), s}}); + }; + NormalizeLabels(p_game->GetPlayers(), get_label, relabel_player); NormalizeLabels(p_game->GetOutcomes(), get_label, set_label); // Action labels are not normalized here: for tree games, ParseNode/ParsePersonalNode // already normalize each infoset's actions individually, at creation, from the raw diff --git a/src/games/game.cc b/src/games/game.cc index 8ea3a1c422..9d72e81c01 100644 --- a/src/games/game.cc +++ b/src/games/game.cc @@ -233,6 +233,48 @@ void GameRep::WriteNfgFile(std::ostream &p_file) const }; } +//------------------------------------------------------------------------ +// GameRep: Players +//------------------------------------------------------------------------ + +void GameRep::RelabelPlayers(const std::map &p_labels) +{ + // Resolve each key to exactly one (personal) player of the game. + std::map assignment; + std::set relabeled; + for (const auto &[old_label, new_label] : p_labels) { + GamePlayerRep *match = nullptr; + for (const auto &player : m_players) { + if (player->GetLabel() == old_label) { + if (match) { + throw ValueException("Player label '" + old_label + "' is ambiguous in this game"); + } + match = player.get(); + } + } + if (!match) { + if (IsTree() && old_label == GetChance()->GetLabel()) { + throw ValueException("The chance player's label cannot be changed"); + } + throw ValueException("No player with label '" + old_label + "' in this game"); + } + assignment[match] = new_label; + relabeled.insert(match); + } + // Replacement labels must be legal, unique against untouched players, and pairwise distinct + std::set targets; + for (const auto &[player, new_label] : assignment) { + CheckPlayerLabel(new_label, relabeled); + if (!targets.insert(new_label).second) { + throw ValueException("Player label '" + new_label + + "' would be duplicated by the relabelling"); + } + } + for (const auto &[player, new_label] : assignment) { + player->m_label = new_label; + } +} + //======================================================================== // MixedStrategyProfileRep //======================================================================== diff --git a/src/games/game.h b/src/games/game.h index c4a8598bba..a0931bac3d 100644 --- a/src/games/game.h +++ b/src/games/game.h @@ -625,7 +625,6 @@ class GamePlayerRep : public std::enable_shared_from_this { Game GetGame() const; const std::string &GetLabel() const { return m_label; } - void SetLabel(const std::string &p_label); bool IsChance() const { return (m_number == 0); } @@ -936,7 +935,9 @@ class GameRep : public std::enable_shared_from_this { void IncrementVersion() { m_version++; } void IndexStrategies() const; /// Validate that p_label is a nonempty, valid, unique label for a player of this game, - void CheckPlayerLabel(const std::string &p_label) const; + /// ignoring the labels of the players in p_ignore. + void CheckPlayerLabel(const std::string &p_label, + const std::set &p_ignore = {}) const; /// Validate that p_label is a nonempty, valid, unique label for an outcome of this game. void CheckOutcomeLabel(const std::string &p_label) const; //@} @@ -1357,6 +1358,8 @@ class GameRep : public std::enable_shared_from_this { auto GetPlayersWithChance() const { return prepend_value(GetChance(), GetPlayers()); } /// Creates a new player in the game, with no moves virtual GamePlayer NewPlayer(const std::string &p_label) = 0; + /// Reassign player labels. Keys of p_labels are current labels; values are their replacements. + void RelabelPlayers(const std::map &p_labels); //@} /// @name Dimensions of the game @@ -1589,7 +1592,8 @@ inline void GameInfosetRep::SetLabel(const std::string &p_label) m_player->CheckInfosetLabel(p_label, {this}); m_label = p_label; } -inline void GameRep::CheckPlayerLabel(const std::string &p_label) const +inline void GameRep::CheckPlayerLabel(const std::string &p_label, + const std::set &p_ignore) const { if (p_label.empty()) { throw ValueException("Player label must not be empty"); @@ -1599,7 +1603,7 @@ inline void GameRep::CheckPlayerLabel(const std::string &p_label) const throw ValueException("Player label must not be the reserved chance player label"); } for (const auto &player : m_players) { - if (player->GetLabel() == p_label) { + if (p_ignore.count(player.get()) == 0 && player->GetLabel() == p_label) { throw ValueException("Player label must be unique within the game"); } } @@ -1619,17 +1623,6 @@ inline void GameRep::CheckOutcomeLabel(const std::string &p_label) const inline bool GameInfosetRep::IsChanceInfoset() const { return m_player->IsChance(); } inline Game GamePlayerRep::GetGame() const { return m_game->shared_from_this(); } -inline void GamePlayerRep::SetLabel(const std::string &p_label) -{ - if (IsChance()) { - throw ValueException("The chance player's label cannot be changed"); - } - if (p_label == m_label) { - return; - } - GetGame()->CheckPlayerLabel(p_label); - m_label = p_label; -} inline GameStrategy GamePlayerRep::GetStrategy(int st) const { m_game->EnsureStrategies(); diff --git a/src/gui/dlgameprop.cc b/src/gui/dlgameprop.cc index 7d11bf1107..8deaa1ef9c 100644 --- a/src/gui/dlgameprop.cc +++ b/src/gui/dlgameprop.cc @@ -136,8 +136,8 @@ PlayerLabelPanel::PlayerLabelPanel(wxWindow *p_parent, GameDocument *p_doc, wxALIGN_CENTER_VERTICAL | wxALIGN_RIGHT); if (player->IsChance()) { - // Chance's label is reserved and can't be changed (GamePlayerRep::SetLabel throws on - // IsChance()) -- just display it. + // Chance's label is reserved and can't be changed (GameRep::RelabelPlayers rejects it + // as a key) -- just display it. gridSizer->Add(new wxStaticText(this, wxID_STATIC, wxString::FromUTF8(player->GetLabel())), 1, wxALIGN_CENTER_VERTICAL); } diff --git a/src/gui/gamedoc.cc b/src/gui/gamedoc.cc index 2a5c377a5f..53065bb2fb 100644 --- a/src/gui/gamedoc.cc +++ b/src/gui/gamedoc.cc @@ -414,9 +414,9 @@ GamePlayer GameDocument::DoNewPlayer() return player; } -void GameDocument::DoSetPlayerLabel(GamePlayer p_player, const wxString &p_label) +void GameDocument::DoRelabelPlayers(const std::map &p_labels) { - p_player->SetLabel(p_label.ToStdString(wxConvUTF8)); + m_game->RelabelPlayers(p_labels); NotifyChanged(GameModificationType::GameLabels); } diff --git a/src/gui/gamedoc.h b/src/gui/gamedoc.h index 2a004a1b5c..d501ad072e 100644 --- a/src/gui/gamedoc.h +++ b/src/gui/gamedoc.h @@ -293,7 +293,8 @@ class GameDocument { void DoSave(const wxString &p_filename, GameSaveFormat p_format); void DoSetTitle(const wxString &p_title, const wxString &p_comment); GamePlayer DoNewPlayer(); - void DoSetPlayerLabel(GamePlayer p_player, const wxString &p_label); + /// Reassign player labels in a single operation; see `Game::RelabelPlayers`. + void DoRelabelPlayers(const std::map &p_labels); /// Declare the strategies of `p_player` in a single operation, covering any combination /// of adding, deleting, reordering, and relabeling strategies. /// @@ -357,10 +358,12 @@ inline GameDocument *NewTableDocument(const std::vector &p_dim) { const Game nfg = NewTable(p_dim); nfg->SetTitle("Untitled Strategic Game"); + std::map labels; int pl = 1; - for (auto player : nfg->GetPlayers()) { - player->SetLabel("Player " + std::to_string(pl++)); + for (const auto &player : nfg->GetPlayers()) { + labels[player->GetLabel()] = "Player " + std::to_string(pl++); } + nfg->RelabelPlayers(labels); return new GameDocument(nfg); } diff --git a/src/gui/gameframe.cc b/src/gui/gameframe.cc index f65902e4f4..0ca2531f4c 100644 --- a/src/gui/gameframe.cc +++ b/src/gui/gameframe.cc @@ -882,15 +882,25 @@ void GameFrame::OnEditGame(wxCommandEvent &) m_doc->DoSetTitle(dialog.GetTitle(), dialog.GetDescription()); TreeRenderConfig style = m_doc->GetStyle(); + std::map labels; for (int i = 0; i < dialog.NumRows(); i++) { const GamePlayer player = dialog.GetPlayer(i); if (player->IsChance()) { style.SetChanceColor(dialog.GetPlayerColor(i)); continue; // chance's label is reserved and can't be changed } - m_doc->DoSetPlayerLabel(player, dialog.GetPlayerLabel(i)); + const std::string newLabel = dialog.GetPlayerLabel(i).ToStdString(wxConvUTF8); + if (newLabel != player->GetLabel()) { + labels[player->GetLabel()] = newLabel; + } style.SetPlayerColor(player->GetNumber(), dialog.GetPlayerColor(i)); } + // Relabeling all players in one call, rather than one at a time, lets two players' + // labels be swapped directly without tripping the duplicate-label check on an + // intermediate state that a per-player rename would pass through. + if (!labels.empty()) { + m_doc->DoRelabelPlayers(labels); + } m_doc->SetStyle(style); } catch (std::exception &ex) { diff --git a/src/pygambit/gambit.pxd b/src/pygambit/gambit.pxd index 6316bb8fe0..785008659b 100644 --- a/src/pygambit/gambit.pxd +++ b/src/pygambit/gambit.pxd @@ -196,7 +196,6 @@ cdef extern from "games/game.h": int IsChance() except + string GetLabel() except + - void SetLabel(string) except +ValueError c_GameStrategy GetStrategy(int) except +IndexError Strategies GetStrategies() except + @@ -319,6 +318,7 @@ cdef extern from "games/game.h": Players GetPlayers() except + c_GamePlayer GetChance() except + c_GamePlayer NewPlayer(string) except +ValueError + void RelabelPlayers(stdmap[string, string]) except +ValueError int NumOutcomes() except + c_GameOutcome GetOutcome(int) except +IndexError diff --git a/src/pygambit/game.pxi b/src/pygambit/game.pxi index 46618b0498..1e281ddbae 100644 --- a/src/pygambit/game.pxi +++ b/src/pygambit/game.pxi @@ -707,8 +707,9 @@ class Game: arrays = list(payoffs.values()) shape = arrays[0].shape g = Game.new_table(shape) - for (player, label) in zip(g.players, payoffs, strict=True): - player.label = label + g.relabel_players( + {player.label: label for player, label in zip(g.players, payoffs, strict=True)} + ) for profile in itertools.product(*(range(s) for s in shape)): for array, player in zip(arrays, g.players, strict=True): g[profile][player] = array[profile] @@ -2819,6 +2820,75 @@ class Game: c_labels.push_back(label.encode("utf-8")) self.game.deref().SetStrategies(resolved_player.player, c_labels) + def relabel_players(self, + labels: typing.Mapping[str, str], + strict: bool = True) -> None: + """Simultaneously reassign the labels of the game's players. + + `labels` maps current player labels to their replacements. The reassignment + is simultaneous, so labels can be swapped directly. Players are not re-ordered: each + relabelled player keeps its position. + + The chance player is not part of the operation: its label is reserved, and a + key of `labels` equal to it raises ``ValueError`` even when `strict` is `False`. + + .. versionadded:: 17.0.0 + + Parameters + ---------- + labels : Mapping[str, str] + A mapping from current player labels to replacement labels. + Entries whose key equals their value are ignored. + strict : bool, default True + If `True`, every key of `labels` must be the label of a player of the game, + and unknown keys raise ``KeyError``. If `False`, unknown keys are ignored. + + Raises + ------ + KeyError + When `strict` is `True`, if a key of `labels` matches no player of the game. + TypeError + If `labels` is not a mapping, or any key or value is not a string. + ValueError + If a key of `labels` matches more than one player; if a key of `labels` + is the label of the chance player; or if any replacement label is empty, + is not a valid label, would result in a duplicate label, or (in an + extensive game) is the reserved label of the chance player. + + See Also + -------- + relabel_actions : Change the labels of actions at an information set. + relabel_strategies : Change the labels of a player's strategies. + """ + if not hasattr(labels, "items"): + raise TypeError( + f"relabel_players(): labels must be a mapping, " + f"not {labels.__class__.__name__}" + ) + current = [player.label for player in self.players] + chance_label = self.players.chance.label if self.is_tree else None + c_labels = stdmap[string, string]() + for old, new in labels.items(): + if not isinstance(old, str) or not isinstance(new, str): + raise TypeError("relabel_players(): labels must map str to str") + if old == chance_label: + raise ValueError("relabel_players(): the chance player's label is reserved") + matches = current.count(old) + if matches > 1: + raise ValueError( + f"relabel_players(): label '{old}' is ambiguous in this game" + ) + if matches == 0: + if strict: + raise KeyError(f"relabel_players(): no player with label '{old}'") + continue + if new == old: + continue + c_labels[old.encode("utf-8")] = new.encode("utf-8") + if c_labels.empty(): + return + self.game.deref().RelabelPlayers(c_labels) + @dataclasses.dataclass class NodeCoordinates: diff --git a/src/pygambit/player.pxi b/src/pygambit/player.pxi index 7236ebe0dc..2916db8ed7 100644 --- a/src/pygambit/player.pxi +++ b/src/pygambit/player.pxi @@ -248,20 +248,19 @@ class Player: @property def label(self) -> str: - """Gets or sets the text label of the player. + """The text label of the player. .. versionchanged:: 17.0.0 A label may now be any well-formed UTF-8 text, not just ASCII; it must still contain no control characters, and must not begin/end with whitespace or have two consecutive whitespace characters. "Whitespace" means any Unicode space separator (e.g. U+00A0 NO-BREAK SPACE), not just the ASCII space. + + The label is now read-only, and must be nonempty and unique among the game's + players; use `Game.relabel_players` to change it. """ return self.player.deref().GetLabel().decode("utf-8") - @label.setter - def label(self, value: str) -> None: - self.player.deref().SetLabel(value.encode("utf-8")) - @property def number(self) -> int: """Returns the number of the player in its game. diff --git a/tests/test_io.py b/tests/test_io.py index 984814e3e1..7a720a470a 100644 --- a/tests/test_io.py +++ b/tests/test_io.py @@ -160,8 +160,7 @@ def test_write_efg_as_nfg(): def test_write_html(): game = gbt.Game.new_table([2, 2]) alice, bob = game.players - alice.label = "Alice" - bob.label = "Bob" + game.relabel_players({alice.label: "Alice", bob.label: "Bob"}) serialized_game = game.to_html() assert isinstance(serialized_game, str) assert "Alice" in serialized_game @@ -171,8 +170,7 @@ def test_write_html(): def test_write_latex(): game = gbt.Game.new_table([2, 2], title="Game title") alice, bob = game.players - alice.label = "Alice" - bob.label = "Bob" + game.relabel_players({alice.label: "Alice", bob.label: "Bob"}) serialized_game = game.to_latex() assert "\\begin{game}" in serialized_game assert "[\\textbf{Alice}][\\textbf{Bob}]" in serialized_game diff --git a/tests/test_outcomes.py b/tests/test_outcomes.py index 65e5f7fa51..b3ebcf6313 100644 --- a/tests/test_outcomes.py +++ b/tests/test_outcomes.py @@ -84,8 +84,7 @@ def test_outcome_index_invalid_type(game: gbt.Game): def test_outcome_payoff_by_player_label(): game = gbt.Game.new_table([2, 2]) pl1, pl2 = list(game.players) - pl1.label = "joe" - pl2.label = "dan" + game.relabel_players({pl1.label: "joe", pl2.label: "dan"}) out1, out2, *_ = list(game.outcomes) out1["joe"] = 1 out1["dan"] = 2 diff --git a/tests/test_players.py b/tests/test_players.py index ccb0135f77..a865aaa901 100644 --- a/tests/test_players.py +++ b/tests/test_players.py @@ -14,7 +14,7 @@ def test_player_count(): def test_player_label(label): game = gbt.Game.new_table([2, 2]) player = next(iter(game.players)) - player.label = label + game.relabel_players({player.label: label}) assert player.label == label @@ -23,7 +23,7 @@ def test_player_label_invalid_raises_valueerror(label): game = gbt.Game.new_table([2, 2]) player = next(iter(game.players)) with pytest.raises(ValueError): - player.label = label + game.relabel_players({player.label: label}) @pytest.mark.parametrize("label", games.UNICODE_LABELS) @@ -31,7 +31,7 @@ def test_player_label_unicode_accepted(label): """Non-ASCII UTF-8 labels are accepted as of #862 (17.0).""" game = gbt.Game.new_table([2, 2]) player = next(iter(game.players)) - player.label = label + game.relabel_players({player.label: label}) assert player.label == label @@ -77,7 +77,7 @@ def test_chance_player_label_cannot_be_changed(): """The chance player's label is reserved ("Chance") and cannot be changed.""" game = gbt.Game.new_tree() with pytest.raises(ValueError): - game.players.chance.label = "Nature" + game.relabel_players({game.players.chance.label: "Nature"}) def test_regular_player_cannot_be_relabeled_to_chance(): @@ -85,14 +85,13 @@ def test_regular_player_cannot_be_relabeled_to_chance(): game.add_player("Alice") player = next(iter(game.players)) with pytest.raises(ValueError): - player.label = "Chance" + game.relabel_players({player.label: "Chance"}) def test_player_index_by_string(): game = gbt.Game.new_table([2, 2]) pl1, pl2 = game.players - pl1.label = "Alphonse" - pl2.label = "Gaston" + game.relabel_players({pl1.label: "Alphonse", pl2.label: "Gaston"}) assert game.players["Alphonse"].label == "Alphonse" assert game.players["Gaston"].label == "Gaston" @@ -113,14 +112,63 @@ def test_set_empty_player_raises_valueerror(): game = games.create_stripped_down_poker_efg() player = next(iter(game.players)) with pytest.raises(ValueError): - player.label = "" + game.relabel_players({player.label: ""}) def test_set_duplicate_player_raises_valueerror(): game = games.create_stripped_down_poker_efg() pl1, pl2, *_ = game.players with pytest.raises(ValueError): - pl1.label = pl2.label + game.relabel_players({pl1.label: pl2.label}) + + +def test_relabel_players_swap(): + game = gbt.Game.new_table([2, 2]) + a, b = (player.label for player in game.players) + game.relabel_players({a: b, b: a}) + assert [player.label for player in game.players] == [b, a] + + +def test_relabel_players_swap_tree(): + game = gbt.Game.new_tree(["Alice", "Bob"]) + game.relabel_players({"Alice": "Bob", "Bob": "Alice"}) + assert [player.label for player in game.players] == ["Bob", "Alice"] + + +def test_relabel_players_duplicate_raises_valueerror(): + game = gbt.Game.new_table([2, 2, 2]) + a, b, _ = (player.label for player in game.players) + with pytest.raises(ValueError): + game.relabel_players({a: b}) + with pytest.raises(ValueError): + game.relabel_players({a: "X", b: "X"}) + + +@pytest.mark.parametrize("bad", ["", " x"]) +def test_relabel_players_bad_label_raises_and_leaves_game_unchanged(bad: str): + game = gbt.Game.new_table([2, 2]) + a, b = (player.label for player in game.players) + with pytest.raises(ValueError): + game.relabel_players({a: "X", b: bad}) + assert [player.label for player in game.players] == [a, b] + + +def test_relabel_players_unknown_label_strictness(): + game = gbt.Game.new_table([2, 2]) + a = next(iter(game.players)).label + with pytest.raises(KeyError): + game.relabel_players({"no-such-player": "X"}) + game.relabel_players({"no-such-player": "X", a: "Y"}, strict=False) + assert next(iter(game.players)).label == "Y" + + +def test_relabel_players_chance_key_raises_even_when_not_strict(): + """The chance player's label is reserved; a key equal to it is an error.""" + game = gbt.Game.new_tree(["Alice"]) + with pytest.raises(ValueError): + game.relabel_players({"Chance": "Nature"}) + with pytest.raises(ValueError): + game.relabel_players({"Chance": "Nature"}, strict=False) def test_strategic_game_add_player():