Skip to content

Correctly parse fchk files without whitespace between values - #408

Merged
tovrstra merged 3 commits into
theochem:mainfrom
tovrstra:fchk-whitespace-fix
Sep 8, 2026
Merged

tovrstra merged 3 commits into
theochem:mainfrom
tovrstra:fchk-whitespace-fix

Conversation

@tovrstra

@tovrstra tovrstra commented Sep 6, 2026

Copy link
Copy Markdown
Member

The test file was generated with Psi4 and has been stripped down below 100k with a new tool tools/stripfchk.py included in this PR.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 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.py to 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.py to 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.

Comment thread iodata/formats/fchk.py
@tovrstra

tovrstra commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

Failing tests will pass after #407 has been merged.

@PaulWAyers PaulWAyers 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.

LGTM.

@tovrstra

tovrstra commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

Thanks for checking. I just reran the tests against the main branch after merging #407. All pass.

@tovrstra
tovrstra merged commit 9f7e800 into theochem:main Sep 8, 2026
11 checks passed
@tovrstra
tovrstra deleted the fchk-whitespace-fix branch September 8, 2026 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants