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

Allow group_by and sort_by to take a list of tags #133

Open
sluijs opened this issue Sep 8, 2022 · 3 comments
Open

Allow group_by and sort_by to take a list of tags #133

sluijs opened this issue Sep 8, 2022 · 3 comments

Comments

@sluijs
Copy link

sluijs commented Sep 8, 2022

For some DICOM datasets I would like to order the slices based on multiple tags. For example, in a dataset with a TemporalPositionIdentifier present, I would like to sort by ImagePositionPatient first, and then using the position identifier. The same applies to group_by too.

Preferably, it would be nice to add support for ascending/descending sorts for each tag too:

# single tag
dm.read(path, sort_by="InstanceNumber")

# multiple tags
dm.read(path, sort_by=["ImagePositionPatient", "TemporalPositionIdentifier"])

# multiple tags + order
dm.read(path, sort_by=[{ "tag": "ImagePositionPatient", "asc": True }, { "tag": "TemporalPositionIdentifier", "asc": False }])

NB: maybe we should consider to set the default sort_by to ImagePositionPatient.

@ad12
Copy link
Owner

ad12 commented Sep 9, 2022

DOSMA supports multi-tag sorting/group_by -

group_by: Union[str, int, Sequence[Union[str, int]]] = "EchoNumbers",
sort_by: Union[str, int, Sequence[Union[str, int]]] = None,

For arguments to sort_by, it may be easier to specify a sort_key argument that allows the user to customize their sorting algorithm

@ad12
Copy link
Owner

ad12 commented Sep 9, 2022

Default for sort_by is InstanceNumber because 1) it is the default value to sort by when writing dicoms and 2) it is scalar value that is easier to sort by. It is unclear what sorting by position would mean in cases where all coordinates in the position are changing (e.g. oblique scans)

@sluijs
Copy link
Author

sluijs commented Sep 9, 2022

DOSMA supports multi-tag sorting/group_by -

Ha. Maybe an option for a callback would be nice for users to define their own sorting algo. Is that what you meant with sort_key?

Default for sort_by is InstanceNumber because 1) it is the default value to sort by when writing dicoms and 2) it is scalar value that is easier to sort by. It is unclear what sorting by position would mean in cases where all coordinates in the position are changing (e.g. oblique scans)

The problem with InstanceNumber is, that doesn't have to correlate with slice position at all. DICOM does not enforce that in the standard IIRC. I'll look into other implementations, but I'm pretty sure most of them use ImagePositionPatient + ImageOrientationPatient.

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