Wrap cclib parser crashes into a clean TSValueError - #112
Merged
Conversation
Verified with a real Gaussian 16 output file (from cclib's own public regression-test data): cclib 1.8.1's Gaussian parser raises an unhandled internal exception on this file's "Leave Link" timing line format (MaxMem=... cpu: ... elap: ...), which read_cclib let escape uncontrolled instead of wrapping into our own TSValueError, unlike every other failure mode in this module (missing dependency, unparseable file, no frequencies). Wrap cclib.io.ccread(...) in a try/except and re-raise as TSValueError with the original exception's type/message, so any cclib parser limitation (this one or a future one, on any supported program) surfaces consistently rather than leaking an arbitrary internal exception. Also add a real, successfully-parsed Gaussian 16 water opt+freq job as a test fixture (parallel real-data coverage to the existing ORCA/Turbomole fixtures), and fix the repo's blanket *.log .gitignore rule (a stock Django-template leftover) shadowing real QM logfile fixtures under tests/data/. Closes #111
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #112 +/- ##
=======================================
Coverage 97.28% 97.29%
=======================================
Files 31 31
Lines 1918 1921 +3
=======================================
+ Hits 1866 1869 +3
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:
|
Review nit: the caught exception type depends on the surrounding logging setup (PQAnalysis's custom logger can raise inside cclib's own logger.error() call before cclib's ValueError re-propagates), not always a bare ValueError. read_cclib's except Exception already handles this correctly either way; only the README's description needed correcting.
galjos
added a commit
that referenced
this pull request
Jul 8, 2026
Small docs-only addition following the ORCA/Gaussian/Turbomole real-software validation work (#108, #110, #112): a brief note in the QM-import usage section stating that `orca_thermo`/`cclib_thermo` are validated against genuine program output, and that unparseable files raise `TSValueError` rather than an arbitrary parser exception. Verified by building the docs locally and visually checking the rendered note (Furo's styled admonition) in the browser. No code changes.
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.
Found by testing against a real Gaussian 16 calculation from cclib's own public regression-test data (cclib/cclib-data) — same approach as the ORCA and Turbomole fixes.
The bug
One real Gaussian 16 file (
mp2_avdz_freq_tight.log, an MP2/aug-cc-pVDZ frequency job) crashes cclib 1.8.1's own Gaussian parser — its "Leave Link" timing line format (MaxMem=... cpu: ... elap: ...) isn't one cclib 1.8.1 handles, and it raises an unhandled internal exception. This is a genuine cclib limitation, not something in this repo — butread_ccliblet that raw exception escape uncontrolled, unlike every other failure mode in the module (missing dependency, unparseable file, no frequencies), which all raise a cleanTSValueError.The fix
Wrap
cclib.io.ccread(...)in a try/except and re-raise asTSValueErrorwith the original exception's type and message. Any cclib parser limitation — this one, or a future one on any supported program — now surfaces consistently instead of leaking an arbitrary internal exception type.Also included
water_neutral_opt_freq.out) as a genuine-data test fixture — parallel real-data coverage to the ORCA/Turbomole fixtures already in the repo, closing the loop on the original "Gaussian, Turbomole, ORCA" ask.*.log.gitignorerule (a leftover from a stock Django template) that was silently excluding real QM logfile fixtures undertests/data/— added a scoped negation.Verification
Ran both real files through the actual code:
Tests: exact-value real-data parsing + full
cclib_thermopipeline for the working file, the real-crash-file regression test, and a cclib-version-independent mocked test of the wrapping logic itself (so coverage doesn't depend on cclib's bug persisting in future releases). Full suite passes (382 passed, 12 pre-existing skips), 100% patch coverage onqm.py.Closes #111