Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reading and writing specialized game formats #470

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

d-kad
Copy link
Contributor

@d-kad d-kad commented Nov 9, 2024

Add following function:

Reading

  • read_efg - reads an .efg file format
  • read_nfg - reads a .nfg file format
  • read_gbt - reads a .gbt file (XML files produced by the GUI)
  • read_agg - reads an .agg file

Writing

  • Game.to_efg - writes an .efg file
  • Game.to_nfg - writes a .nfg file
  • Game.to_gbt - writes a .gbt file
  • Game.to_html - writes out HTML tables
  • Game.to_latex - writes a .tex file

@tturocy
Copy link
Member

tturocy commented Nov 14, 2024

This is an excellent start. Some suggestions for next steps:

  • There's a fair amount of code duplication across these - and because we may sometime add new formats it would be even more helpful to avoid this. I think there's opportunities for eliminating some of this on both the reading and writing sides.
  • For writing, I'd suggest we want to break out new WriteXXXFile wrappers rather than using the existing WriteGame, just to keep things parallel. Also for writing, I think it should be possible now to use std::string as arguments instead of char * - the char * dates from a long time ago and Cython has massively improved its support for working with C++ types since then.
  • We should have some test suite entries for these.
  • A deprecation warning should be added to the existing functions' docstrings.
  • Finally, these should be added as appropriate to the documentation. The existing uses of read and write calls should be changed to use these, at minimum. Further we could consider whether there should be a section in the user guide discussing ways of reading/writing games.

@d-kad
Copy link
Contributor Author

d-kad commented Dec 6, 2024

Two questions at this point:

  1. WriteGbtFile function is commented as including gui/gamedoc.h in util.h causes compilation errors.
    Is there an easy fix for this or should we leave it aside for v16.3?
  2. Regarding the documentation, are there any suggestions on tools generating documentation for pygambit.api.rst or should it be handled manually?

@tturocy
Copy link
Member

tturocy commented Dec 19, 2024

Very nice on the refactoring.

Yes, we can't at the moment implement write_gbt. We weren't supporting it before, so we don't need to include it now. It's a rather separate discussion to have whether we even want it at all, and if so what arguments it should take. So it deserves its own separate issue.

For marking the deprecation and also flagging functions as being new, see the numpydoc style at https://numpydoc.readthedocs.io/en/latest/format.html - also consider See also entries for the new functions

pygambit.api.rst is a manually-managed index file, so we've got to put the entries there wherever they belong.

For tests, use parameterised text fixtures (see other test files for examples). This way you can run them with more than one game which would be a good idea - in particular nontrivial games should be tested.

For efg/nfg, a good test to focus on is whether the game makes the round trip from game to file back to game correctly.

For validating the HTML - it does seem like we ought to do some sort of check but I'm not quite sure we should have our own parser, that feels something like neither here nor there. We might simply at this point confirm that it runs and put the question of the best way to regression test files we don't read in, off for another day.

@d-kad d-kad force-pushed the issue_357 branch 3 times, most recently from afa6fcf to df470a5 Compare December 21, 2024 13:40
…zed formats

Reading
* `read_efg` - reads an .efg file format
* `read_nfg` - reads an .nfg file format
* `read_gbt` - reads a .gbt file (XML files produced by the GUI)
* `read_agg` - reads an action-graph games file format

Writing
* `Game.to_efg` - writes an .efg file
* `Game.to_nfg` - writes an .nfg file
* `Game.to_html` - writes out HTML tables
* `Game.to_latex` - writes a .tex file

New tests in test_io.py
@d-kad
Copy link
Contributor Author

d-kad commented Dec 21, 2024

Tests on reading and writing nfg files are skipped as the input and output text files are not identical when using C++ read and write functions. One of the efg games is skipped for the same reason.

Shall we leave it at that for now?

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