Harden _parse_fukui's table-end detection - #125
Merged
Conversation
xtb writes Fukui data only to stdout (verified: a clean-directory --vfukui run writes no dedicated file for it), so this parser is unavoidably stdout-scraping -- but it was only checking token count to detect the end of the table, so a coincidental 4-token line right after it would get silently misparsed as a data row. Now also checks the label matches "digits then letters" and the last 3 tokens parse as floats before accepting a row.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #125 +/- ##
=======================================
Coverage 97.39% 97.40%
=======================================
Files 32 32
Lines 1995 2001 +6
=======================================
+ Hits 1943 1949 +6
Misses 52 52
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
`_parse_fukui` (added in #123) only checked token count (`len(tokens) != 4`) to detect the end of the `Fukui functions:` table. A code review flagged this as an "acceptable risk" of stdout-scraping, but on a second look it's a cheap fix, not something to just accept.
Verified xtb writes this data *only* to stdout: a clean-directory `--vfukui` run writes no dedicated file for it, only `charges`/`wbo`/`xtbrestart`/`xtbtopo.mol` -- so stdout-scraping itself isn't avoidable. But the end-of-table check can be tightened: a coincidental 4-token line right after the real table (before the blank line that actually ends it) would previously get silently misparsed as a data row.
The fix
Also verify the label matches "digits then letters" (`^(\d+)([A-Za-z]+)$`) and the last 3 tokens parse as floats before accepting a row; either check failing now correctly ends the table instead of raising deep inside a malformed row.
Verification
Two new regression tests reproduce the exact failure mode this fixes (a coincidental 4-token line with a bad label, and one with non-numeric values), plus a test for multi-digit atom indices with multi-letter element symbols (e.g. `14Cl`). Full suite: 421 passed, 10 skipped with a real `xtb` binary. New code fully covered (only remaining gap in the file pre-dates this change). Docs build clean.
Closes #124