Decode JCAMP-DX parameter files as UTF-8, not as the locale encoding - #203
Merged
Merged
Conversation
gdevenyi
force-pushed
the
fix/jcampdx-utf8-encoding
branch
from
August 8, 2026 19:50
8275c9c to
50ebee8
Compare
`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
Spec 2.2 says ParaVision 360 writes parameter files as UTF-8 and that a reader must decode them as UTF-8. Every read and the write instead used Python's default text encoding, i.e. locale.getpreferredencoding(False). Under LC_ALL=C -- a container, a CI runner, a batch scheduler -- reading a PV360 parameter-map visu_pars raised UnicodeDecodeError, which read_jcampdx then reported as the misleading "is not a text file". Under a single-byte code page it would be worse: the file decodes without error into the wrong string. Name the encoding once, next to the format constants it belongs with, and pass it at the four open sites. Split the error handling so a genuine I/O failure no longer claims the file is not text. Closes isi-nmr#196 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017SNrm3jFhpTPShP8xGkePC
gdevenyi
force-pushed
the
fix/jcampdx-utf8-encoding
branch
from
August 8, 2026 20:50
50ebee8 to
5ca2726
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 #196.
Spec 2.2:
All four
open()sites injcampdx.pyused Python's default text encoding, so the format's encoding came from the process locale.Change
ENCODING = "utf-8"named next to the other format constants, passed atJCAMPDX.version,load_parameter,read_jcampdxandwrite.read_jcampdxno longer reports a valid UTF-8 file as "is not a text file";UnicodeDecodeErrorandOSErrornow get distinct messages.test/synthetic.py:write_jcampdxwrites UTF-8 too, so a fixture can carry the strings ParaVision actually writes.Test
test_utf8_values_survive_a_non_utf8_localewrites the PV360 T2-mapVisuFGElemCommentvalues (σ of Signal Intensity,Fit χ²) and reads them back in a subprocess underLC_ALL=Cwith UTF-8 mode off. Without the fix that subprocess exits non-zero withJcampdxFileError: ... is not a text file; with it the strings round-trip. No vendor data is touched.Suite: 2141 passed, 12 skipped (baseline 2140/12 — the one new test).