python-chess
python-chess copied to clipboard
Handle multiple comments on a move/variation/game
Some chess programs that write PGN files (including lichess.org and others mentioned in the linked issue) will write multiple consecutive braced comments--e.g., 1. e4 { A very common opening } { Perhaps too common ... } { [%clk 0:00:01] }
. Previously, when read by chess.pgn.read_game()
, these comments would be joined into a single comment with a single space between them. This PR creates a new class to read, store, and write these comments while keeping them separate. The user interface is kept backwards compatible by using @property
decorators to allow assignment and comparison of bare strings and string lists to game node comments.
Closes #946
Hi. Good idea.
I think changing the property type in this way is not quite backwards compatible, even if __str__
is provided. Maybe something like
class GameNode:
comments: List[Comment]
@property
def comment(self) -> str: ...
@comment.setter
def comment(self, value: str) -> None: ...
could work?
Or maybe it's also just time to consider 2.x at some point.
I tried to make GameNodeComment
backwards compatible in terms of assignment and writing to a PGN file. The breaking change comes from getting comments from a node (with some allowances for comparing a single comment to a single string), since I didn't want users to accidentally miss comments. What would GameNode.comment
return, just the first comment?
Saving this until 2.x would make things much easier. A new class wouldn't even be needed.
class GameNode:
comments: list[str]
If you want to save this PR until 2.x, this would be an easy change.
In 1.x, GameNode.comment
would have to be the concatenation of all comments, to match the current behavior.
That should do it. GameNode.comments
returns the GameNodeComment
(a list[str]
essentially), and GameNode.comment
returns a single str
formed from joining the strings with spaces.
Do you want to save this functionality for v2.0? I can make a new PR (not backwards compatible) that would be much simpler than this one--basically replacing all comment: str
members of GameNode
and ChildNode
with comments: list[str]
and changing the PGN comment functions.
Let's do 2.x. So it would contain "small" breaking changes and clean-ups, and more ambitious plans and far-reaching changes would be pushed back to 3.x.
Updated changes:
- Rename
GameNode
comment field fromcomment
tocomments
andstarting_comment
tostarting_comments
. - Adapt methods that process comments (e.g.,
eval()
andset_eval()
). - Adapt PGN reading and writing to handle moves/games with multiple comments.
The change from comment
to comments
is the breaking change. Assigning to node.comment
no longer has any effect. I figured that using a plural name emphasizes that the field should be a list. Plus, assigning a string comment instead of a list of strings will cause problems with comment processing methods (Item 2, above).
Looks like the release of mypy 1.11 is catching new errors.
Fixed via #1100. Targetting one final 1.x release before making the jump to 2.x.
Should extra whitespace be preserved when reading comments? To ask another way, what was the reason for this commit? I would think that a newline in the middle of a curly brace comment was put there by word wrapping and so would not be a significant part of the comment. Is there a problem with formatting imported comments with the equivalent of
comment = " ".join(comment.split())
to replace all whitespace with single spaces?