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

Change default setting for group_by in the DICOM reader #132

Open
sluijs opened this issue Sep 8, 2022 · 1 comment
Open

Change default setting for group_by in the DICOM reader #132

sluijs opened this issue Sep 8, 2022 · 1 comment

Comments

@sluijs
Copy link

sluijs commented Sep 8, 2022

The group_by argument of DicomReader currently defaults to EchoNumbers, which is not commonly present in DICOM files.

Typically, I would only load one DICOM study and not use grouping at all. I would suggest to default to None and to disable grouping that scenario (i.e., not returning a list of MedicalVolume, but a single instance).

@sluijs sluijs changed the title Change default setting for group_by in the DICOM reader Change default setting for group_by in the DICOM reader Sep 8, 2022
@EAlexWaters
Copy link

May I suggest instead having the group_by argument default to EchoTime?

This accomplishes:

  • Makes DOSMA not crash when you try to load DICOMS that don't implement EchoNumbers
  • It's a mandatory tag, so all MRI DICOM files should have it
  • Handles the case where you only have one echo time and don't need grouping

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

2 participants