Correctly parse fchk files without whitespace between values - #408
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The new float tokenizer accepts D/d exponents via the regex but still passes tokens directly to float(), which will fail on Fortran-style D exponents.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR improves IOData’s FCHK reader robustness for real-valued array fields by correctly tokenizing floating-point numbers even when adjacent values are not separated by whitespace (as can happen with large exponents), and adds a minimized Psi4-generated FCHK fixture plus a helper script to keep test data under the repository size limit.
Changes:
- Update
iodata/formats/fchk.pyto extract float array values via a regex tokenizer instead of whitespace splitting. - Add a Psi4-derived FCHK test fixture and a new regression test that validates correct parsing (including orthonormal MO checks).
- Introduce
tools/stripfchk.pyto strip irrelevant FCHK fields while preserving formatting quirks needed for parser regression tests.
File summaries
| File | Description |
|---|---|
tools/stripfchk.py |
New utility to strip/retain selected FCHK fields to keep test fixtures small while preserving raw formatting. |
iodata/test/test_fchk.py |
New regression test covering Psi4 FCHK parsing when float values are adjacent without whitespace. |
iodata/test/data/psi4_hcch.fchk |
New stripped Psi4-generated fixture exhibiting adjacent-float formatting edge cases. |
iodata/formats/fchk.py |
Switch float-array tokenization to a regex-based approach to handle missing whitespace between values. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Failing tests will pass after #407 has been merged. |
|
Thanks for checking. I just reran the tests against the main branch after merging #407. All pass. |
The test file was generated with Psi4 and has been stripped down below 100k with a new tool
tools/stripfchk.pyincluded in this PR.