Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Addition of MAGNITUDE to ImageType of Siemens data #851

Closed
mharms opened this issue Aug 14, 2024 · 4 comments
Closed

Addition of MAGNITUDE to ImageType of Siemens data #851

mharms opened this issue Aug 14, 2024 · 4 comments

Comments

@mharms
Copy link
Collaborator

mharms commented Aug 14, 2024

I noticed that the current development branch has added MAGNITUDE to the ImageType tag of Siemens data:

v1.0.20240812
"ImageType": ["ORIGINAL", "PRIMARY", "M", "NONE", "MAGNITUDE"],

v1.0.20240202
"ImageType": ["ORIGINAL", "PRIMARY", "M", "NONE"],

Is that intentional? I noticed this recent commit 08aeeef, but that is framed as only being relevant to GE.

On the one hand, this is a minor change, but on the other hand I think that ImageType should reflect the actual DICOM entry, and not change the behavior for Siemens data unless such change is actually necessary.

@bpinsard: Why was this change necessary for GE data, and why couldn't heudiconv have been adapted instead of hacking dcm2niix to accommodate heudiconv?

@neurolabusc
Copy link
Collaborator

@mharms I have updated the detection, so the term "MAGNITUDE" will not be appended if either "MAGNITUDE" or "M" already exist in the list of ImageType parameters. It does seem worth explicitly denoting that an image is a magnitude image if the DICOM explicitly declares this but it is not listed in the ImageType.

@bpinsard
Copy link

You are entirely right, that shouldn't modify the ImageType when the info is not missing in the first place.
Surprisingly, if we check the 2 following lines, that is what happened for the "PHASE" in previous versions.

For GE data, that info is not in the ImageType, which is why it needs to be inserted (to avoid ambiguity, notably for dcm2bids that do not parse the dicoms, but use only sidecar contents).

I would be all for a new generated standardized tag in the sidecar to store that info in a more standardized way and separated from the rest of the ImageType labels.

@bpinsard
Copy link

@neurolabusc
Copy link
Collaborator

@bpinsard the latest commit only adds terms like PHASE, REAL, etc if the term (e.g. "PHASE") or its alias (e.g. "P") does not exist.

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

No branches or pull requests

3 participants