Skip to content

Preserve PGN tags on load -> save - #6

Merged
BrandonZacharie merged 1 commit into
mainfrom
claude/review-chess-py-wktEg
May 24, 2026
Merged

Preserve PGN tags on load -> save#6
BrandonZacharie merged 1 commit into
mainfrom
claude/review-chess-py-wktEg

Conversation

@BrandonZacharie

Copy link
Copy Markdown
Owner

Summary

Loading a PGN, saving it back, and re-loading the saved file used to lose everything except the moves — Event / Site / Date / Round / White / Black / Result all reset to the engine's defaults. For anyone loading a famous game and saving it back, that's a surprise. This PR preserves the metadata across a load → save round-trip.

Changes

  • Game.pgn_tags: Dict[str, str] — new public field, defaults to {} on a fresh game; copy semantics in the from-existing-game ctor branch.
  • PGNFile.game() — populates pgn_tags from the pgnparser.PGNGame attributes for the Seven Tag Roster plus Annotator, PlyCount, TimeControl, Time, Termination, and Mode. Only non-empty values are stored.
  • Game.save_pgn() — tags now layer as built-in defaults < self.pgn_tags < caller headers=. So:
    • Load → save reproduces the original metadata.
    • A fresh game still uses the engine's defaults (Casual Game / Chess.py vX.Y.Z / today / ? / ? / ? / derived Result / counted PlyCount).
    • A caller passing headers= can still override anything (used by callers, never invoked from the UI today, but documented behavior).

The PGN-recorded Result wins over the derived one when both are present — important for games that ended by resignation or agreement (and never reached an on-board mate), where game.result() would otherwise say *. For a brand-new game played to mate, the derived Result still gets used (no pgn_tags entry to override it).

Scope notes

JSON save/load is intentionally untouched. pgn_tags lives on the Game object but isn't serialized to JSON, so a PGN → JSON → PGN cycle still loses the tags. Threading pgn_tags through serializable() / parse_json_file() is the natural follow-up but adds a JSON format change and tests on the JSON path, so it didn't make sense to bundle here.

Tests

Five new tests in tests/test_game.py:

  • Fresh Game() has pgn_tags == {}.
  • PGNFile.game(0) against test1.pgn populates all seven STR tags correctly.
  • Load → save reproduces every STR tag including the recorded Result "1/2-1/2" (the Fischer–Spassky draw — note game.result() would say * since no mate occurred).
  • Caller headers= overrides both defaults and loaded tags.
  • A brand-new Game still emits the Casual Game / ? / ? defaults.

Test plan

  • make test — fast suite (213 passed, 2 deselected) in ~1.4 s.
  • make test-all — full sweep including Carlsen/Nakamura passes in ~7:42. Verified locally.
  • coverage report after make test shows TOTAL 100%.
  • Manual: load tests/test1.pgn, save as PGN to a new path, diff the two — Event/Site/Date/White/Black/Result/Round all match the input.
  • Manual: start a new game, save as PGN — Event is Casual Game, White/Black are ?.

https://claude.ai/code/session_01RnQvxtXfn9JFTN8jpNyriL


Generated by Claude Code

Loading a PGN, saving it back, and re-loading the saved file lost
everything except the moves — Event / Site / Date / Round / White /
Black / Result all reset to the engine's defaults ('Casual Game' /
'Chess.py vX.Y.Z' / today / '?' / '?' / '?' / derived). For anyone
loading a famous game and saving it back that's a surprise.

- Adds a public ``pgn_tags: Dict[str, str]`` field on Game (and a
  matching copy in the from-existing-game ctor branch). Empty by
  default.
- PGNFile.game() now populates it from the pgnparser PGNGame's
  attributes for the seven-tag roster plus Annotator, PlyCount,
  TimeControl, Time, Termination, and Mode. Only non-empty values
  are stored.
- Game.save_pgn() layers tags as: built-in defaults < self.pgn_tags <
  caller's ``headers`` argument. So a load -> save now reproduces
  the original metadata; a fresh game still uses the defaults; and
  a caller can still override anything by passing headers=.
- The PGN-recorded Result wins over the derived one — important for
  games that ended by resignation or agreement (and never reached an
  on-board mate), where game.result() would otherwise say '*'.

JSON save/load is intentionally untouched; pgn_tags lives on the
Game object but isn't serialized, so a PGN -> JSON -> PGN cycle
still loses the tags. Adding a JSON field is the natural follow-up.

Tests: empty default on fresh game; PGNFile.game() captures all
seven tags from test1.pgn; save_pgn round-trip preserves them and
prefers the PGN Result over derived; caller headers= still wins;
brand-new games still emit defaults.
@BrandonZacharie
BrandonZacharie merged commit 1b138f8 into main May 24, 2026
3 checks passed
@BrandonZacharie
BrandonZacharie deleted the claude/review-chess-py-wktEg branch May 24, 2026 23:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants