Skip to content

Resolve rawdata word type from GO_raw_data_format/ACQ_word_size + BYTORDA (no version gate)#39

Merged
headmeister merged 1 commit into
isi-nmr:masterfrom
gdevenyi:fix/pv360-rawdata-wordtype-coverage
Jul 22, 2026
Merged

Resolve rawdata word type from GO_raw_data_format/ACQ_word_size + BYTORDA (no version gate)#39
headmeister merged 1 commit into
isi-nmr:masterfrom
gdevenyi:fix/pv360-rawdata-wordtype-coverage

Conversation

@gdevenyi

@gdevenyi gdevenyi commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

What this does

Resolve the rawdata word type purely from GO_raw_data_format / ACQ_word_size + BYTORDA, with no ACQ_sw_version gate.

Previously numpy_dtype required ACQ_sw_version to be in a hardcoded whitelist (a <PV 5.1>/<PV 6.0>/<PV 6.0.1>/<PV-7.0.0> list for the GO_* branches, a PV-360 list for the ACQ_word_size branch), and the ACQ_word_size (job-based / PV360) path only covered _32_BIT little-endian. The raw word type is fully determined by the word-format and byte-order parameters and does not depend on the ParaVision version, so those version conditions were both redundant and a source of MissingProperty failures on any unlisted version.

This reframes the original "add PV360 16-bit/big-endian branches" change following the discussion in #41 (thanks @headmeister).

Change

  • Drop the ACQ_sw_version condition from every numpy_dtype branch.
  • Cover all word-size × byte-order combinations for the ACQ_word_size fallback — adds _32_BIT/big, _16_BIT/little, _16_BIT/big alongside the existing _32_BIT/little.
  • GO_* branches are matched first (authoritative when the GO subclass is present); the ACQ_word_size branches are the fallback for data with no GO_* subclass. PV360 data carries no GO_raw_data_format, so the GO branches simply don't match it and the ordering is safe.

Genuinely version-dependent selection is left alone — in particular job_desc, whose ACQ_jobs struct layout really does differ between PV-360 releases, keeps its version conditions (including <PV-360.1.1>). This PR deliberately does not touch the PV-360 version enumeration discussed in #41.

Coverage / datasets

  • PV6 rawdata.jobN (has GO_raw_data_format) and PV360 rawdata.jobN (has ACQ_word_size, no GO_*) both resolve as before on the data I have — all _32_BIT/little: MRIReco.jl PV360 CS FLASH, Bruker PV360_StdData v3.6, Bruker PCI PV360 standard datasets.
  • I could not find any public dataset with ACQ_word_size=_16_BIT or BYTORDA=big, so the new 16-bit / big-endian branches are not exercised by a public fixture; they close a documented ACQ_word_size/BYTORDA gap (_16_BIT is listed in the Parameter Reference and appears in the ParaVision install source).

🤖 Generated with Claude Code

The PV-360 rawdata numpy_dtype only resolved for ACQ_word_size=_32_BIT with
BYTORDA=little. ParaVision's ACQ_word_size also allows _16_BIT (D02 PvParams),
and BYTORDA may be big; those combinations previously matched no branch and
failed with MissingProperty. Add the int16 and big-endian variants next to the
existing int32/little entry, keeping the same ACQ_sw_version gate.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 22, 2026 16:25

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@headmeister
headmeister merged commit 73cc7cd into isi-nmr:master Jul 22, 2026
8 checks passed
@gdevenyi gdevenyi changed the title Add PV360 16-bit and big-endian raw word-type branches Resolve rawdata word type from GO_raw_data_format/ACQ_word_size + BYTORDA (no version gate) Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants