Carry on when an optional parameter file is present but unreadable - #207
Merged
headmeister merged 2 commits intoAug 10, 2026
Merged
Conversation
`ruff format --check` reported twelve files as unformatted, so any change touching one of them dragged unrelated reflow into its diff. Run the formatter once, on its own, so subsequent changes show only what they actually change. Formatting only: no behaviour changes, and `ruff check` is clean before and after. The suite is unchanged at 2140 passed, 12 skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017SNrm3jFhpTPShP8xGkePC
_read_parameters tolerated a missing optional file but caught only FileNotFoundError, so anything else -- an empty, truncated or unsupported-version JCAMP-DX file -- propagated and aborted the whole dataset. The reco lookup for a fid did the same through `with suppress(FileNotFoundError)`. Spec 1.3 makes every PROCNO entry optional in the manuals' own framing, and notes that derived reconstructions carry 2dseq and visu_pars with no reco at all; a fid is read from acqp and method. A zero-byte pdata/1/reco -- what an aborted or interrupted reconstruction leaves behind, which is exactly the case a raw-data reader exists to rescue -- nevertheless made a complete raw acquisition unloadable. Five experiments in the local corpus are affected. Widen the tolerated set to the JCAMP-DX error family, keep it fatal for the files DEFAULT_STATES declares required, and warn rather than raise so the omission is visible. Absence stays silent, as before. Closes isi-nmr#199 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017SNrm3jFhpTPShP8xGkePC
gdevenyi
force-pushed
the
fix/optional-parameter-files
branch
from
August 8, 2026 20:51
7ea840b to
374e9c1
Compare
Contributor
Author
|
Rebased onto #213 ( Integration check across all eleven conformance PRs merged together: 2164 passed, 12 skipped; corpus load test 3202 → 3207 of 3478 with zero newly broken datasets. |
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
2 participants
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.
Fixes #199.
_read_parameterstolerated a missing optional file but caught onlyFileNotFoundError, so anything else — an empty, truncated or unsupported-version JCAMP-DX file — propagated and aborted the whole dataset. Therecolookup for afiddid the same viawith suppress(FileNotFoundError).Spec 1.3:
A
fidis read fromacqp+method, yet a zero-bytepdata/1/recomade a complete raw acquisition unloadable:Five experiments in the local corpus are affected. Remove the empty
recoand the samefidloads as(256, 256, 1, 1, 1).Change
OPTIONAL_PARAMETER_ERRORSnames the family:FileNotFoundError,InvalidJcampdxFile,JcampdxFileError,JcampdxVersionError.DEFAULT_STATES[type]["parameter_files"]stay fatal; optional ones are skipped.RuntimeWarning) unless the file is simply absent, so the omission is visible rather than silent.Tests
test_an_unreadable_optional_parameter_file_does_not_abort_the_load— builds a synthetic PV6 experiment, records the reference array, drops a zero-bytepdata/1/reconext to it, and asserts the data is unchanged and the warning is raised. Fails on master.test_a_missing_required_parameter_file_is_still_fatal— guards the other direction.Suite: 2142 passed, 12 skipped.