Skip to content

Remove integer indexing from game collections in pygambit#942

Merged
tturocy merged 24 commits into
gambitproject:masterfrom
d-kad:indexing
Jun 18, 2026
Merged

Remove integer indexing from game collections in pygambit#942
tturocy merged 24 commits into
gambitproject:masterfrom
d-kad:indexing

Conversation

@d-kad

@d-kad d-kad commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Description

Game object collections in pygambit are now indexed by string label only.

  • Integer indexing is removed from Game.players, Game.outcomes, Game.strategies, Game.infosets,
    Game.actions, Player.strategies, Player.infosets, Player.actions, Infoset.actions, Infoset.members,
    and Node.children. Indexing by an integer raises TypeError.
    Positional access remains available by iterating a collection (e.g. pl1, pl2 = game.players).
    Indexing a Game by a contingency (e.g. game[0, 1]) is unchanged.
  • Label lookup is by exact match.
    A label matching no object uniformly raises KeyError (Node.children previously raised ValueError).
  • Lookup is centralised in a new _resolve_by_label helper; all the collection __getitem__ methods use it.
    The label parameter is type-hinted str, so Cython rejects non-string indices at the call boundary.
  • The test suite (including the tutorial notebooks) is fully migrated to label-based access (~460 affected tests),
    with new tests added pinning the integer-rejection across all collections
    (COLLECTION_GETTERS in tests/test_game.py).

d-kad added 19 commits June 9, 2026 12:40
iteration and tuple unpacking in quickstart, stripped-down poker, and
OpenSpiel tutorials.
Players, outcomes, strategies, infosets, actions, infoset
members and Node.children  are indexed by str label only.
Integer indexing raises TypeError with a 16.7.0 migration message;
unknown labels raise KeyError; lookup is by exact match, with no
whitespace stripping.  Contingency indexing on Game is unchanged.
@d-kad d-kad requested review from rahulsavani and tturocy June 12, 2026 07:04
@review-notebook-app

Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@d-kad

d-kad commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Design question — should label lookup strip whitespace, or match exactly?

Before this PR, the collection __getitem__s stripped the lookup string before comparing:

matches = [x for x in self if x.label == index.strip()]

This appears at 10 sites (game.pxi ×5, player.pxi ×3, infoset.pxi ×2),
so e.g. g.players[" Alice "] would find the player labelled "Alice".
The most recent new Node.children semantics in #588, matches labels verbatim.

In this PR I matched labels exactly. ~~ (superseded — see update)

UPD: @rahulsavani and I agreed label lookup should keep stripping surrounding
whitespace from the query (stored labels are matched as-is).

@d-kad d-kad self-assigned this Jun 12, 2026
@d-kad d-kad added python Items which involve coding in Python enhancement cython Items which involve coding in Cython labels Jun 12, 2026
@d-kad d-kad added this to the gambit-16.7.0 milestone Jun 12, 2026

@tturocy tturocy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few places where we should be using Cython type hints.

Comment thread src/pygambit/game.pxi Outdated
Comment thread src/pygambit/gambit.pyx
@tturocy tturocy merged commit 193a179 into gambitproject:master Jun 18, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cython Items which involve coding in Cython enhancement python Items which involve coding in Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants