Skip to content

Add Twix, MRS and PMU raw data readers - #2

Merged
tclose merged 7 commits into
mainfrom
james/twix-raw-data
Feb 23, 2026
Merged

tclose merged 7 commits into
mainfrom
james/twix-raw-data

Conversation

@jamesdidathing

Copy link
Copy Markdown
Contributor
  • Added .rda (MRS spectroscopy) format: validates the >>> Begin of header <<< marker and parses metadata from the text header
  • Added .puls (PMU) format: validation through extension with metadata parsing from the footer section
  • Added .dat (twix k-space) format: validates VB/VD/VE version (replicating same check used in twixtools), exposes measurement count, and reads protocol headers via twixtools
  • Added tests for all 3 new formats using mock data

Reviewers:
Run tests and ensure they pass correctly, and review the code and logic

@codecov

codecov Bot commented Feb 23, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.88889% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.85%. Comparing base (12cfb29) to head (a154002).
⚠️ Report is 12 commits behind head on main.

Files with missing lines Patch % Lines
...tras/fileformats/extras/vendor/siemens/medimage.py 81.39% 8 Missing ⚠️
fileformats/vendor/siemens/medimage/twix.py 90.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main       #2      +/-   ##
==========================================
+ Coverage   86.60%   86.85%   +0.25%     
==========================================
  Files           5        8       +3     
  Lines         224      312      +88     
==========================================
+ Hits          194      271      +77     
- Misses         30       41      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tclose tclose left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @jamesdidathing, it looks great! Just have a few minor comments and suggestions on naming conventions

from fileformats.medimage.dicom import DicomImage
from fileformats.vendor.siemens.medimage import (
SiemensPuls,
SiemensRda,

@tclose tclose Feb 23, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re naming convention, the format of the raw data files should be consistent with the console software platform, which for the Cima X is called "Syngo MR XA". FileFormats automatically maps Python class names to "mime-like" string with _ -> . and [A-Z] -> -[a-z] so fileformats.vendor.siemens.medimage.SyngoMr_Xa_Puls -> medimage/vnd.siemens.syngo-mr.xa.puls.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Didn't realise this, changed them now

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actually, I should have written SyngoMr_Xa_Puls with the lowercase r to match the convention. Would you mind find/replacing that?

SyngoMi_Vr20b_Normalisation,
SyngoMi_Vr20b_Parameterisation,
SyngoMi_Vr20b_CtSpl,
TwixRawData,

@tclose tclose Feb 23, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

SyngoMr_Xa_Twix is probably a good name

binary = True

@validated_property
def version_is_ve(self) -> bool:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The Cima is XA (the next version after VE). Do the twixtools support it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So from what I can tell, VE and XA have the same underlying structure so it should handle correctly. It works with the example file you sent over to me, but might be worth testing with some more files?

The method name/docstring I can change too if its confusing.

@tclose

tclose commented Feb 23, 2026

Copy link
Copy Markdown
Contributor

NB: I will create a separate PR to fix the mac-os ci/cd problem

@tclose

tclose commented Feb 23, 2026

Copy link
Copy Markdown
Contributor

NB: I will create a separate PR to fix the mac-os ci/cd problem

This is done now. Could you rebase on main?

@tclose
tclose merged commit fa20772 into main Feb 23, 2026
13 checks passed
@tclose
tclose deleted the james/twix-raw-data branch September 28, 2026 07:12
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.

2 participants