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

Add MOOSE segmentation JSON (dataset.json) format support directly #16

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

prabathbr
Copy link

@prabathbr prabathbr commented Aug 21, 2023

This patch will use dataset.json from a MOOSE segmentation directly. It will convert the input JSON data to suit nifti2dicom input JSON format on the fly.

Usage:
Added new argument "-m" or "--moose", conversion will apply when it is true. -m True

parser.add_argument("-m", "--moose", type=str, choices=['True', 'False'], required=False, default='False')

Sample Input dataset.json :

{'name': 'Dataset001_PUMA', 'description': '', 'reference': '', 'licence': 'hands off!', 'release': '0.0', 'labels': {'background': 0, 'legs': 1, 'body': 2, 'head': 3, 'arms': 4}, 'numTraining': 121, 'file_ending': '.nii.gz', 'channel_names': {'0': 'CT'}}

Sample conversion at organ_index:

{'1': 'legs', '2': 'body', '3': 'head', '4': 'arms'}

@LalithShiyam
Copy link
Collaborator

Hi @prabathbr, cool idea. However, I would prefer to keep this as generic as possible and not steering it towards moose. 😄

@prabathbr
Copy link
Author

Hi @prabathbr, cool idea. However, I would prefer to keep this as generic as possible and not steering it towards moose. 😄

Thanks, I have made this additional tag "False" as default so that it would work normal if -m True is not added to the command line.

Would it be possible to merge this ? I thought it would be a nice option to have when MOOSE is used in the upstream with nifti2dicom, so that no external script is required to to parse MOOSE output JSON before feeding to nifit2dicom. 😄

@LalithShiyam
Copy link
Collaborator

I see/understand your point, I did discuss with @Keyn34, for his thoughts as well. So, from our perspective, it's not good to add special tags (specific for moose) when we use this for 5x tools internally. It's just a design choice. Also the conversion is simple - we don't need an extra tag. Sorry.

@Keyn34
Copy link
Collaborator

Keyn34 commented Aug 22, 2023

Hey @prabathbr, thank you for raising this and your input. We want to keep our tools as simple, stand-alone and encapsulated as possible. Although I understand your rationale, it would introduce a higher complexity in maintaining the tools as they would gradually become co-dependent.

I suggest building an interface tailored towards your demand. This is what I usually do when I want to use tools subsequently and intertwine. :) That way, you also have better control and can adjust things as you see fit.

Thank you for your effort, and I hope our perspective is understandable. :)

@prabathbr
Copy link
Author

Hey @prabathbr, thank you for raising this and your input. We want to keep our tools as simple, stand-alone and encapsulated as possible. Although I understand your rationale, it would introduce a higher complexity in maintaining the tools as they would gradually become co-dependent.

I suggest building an interface tailored towards your demand. This is what I usually do when I want to use tools subsequently and intertwine. :) That way, you also have better control and can adjust things as you see fit.

Thank you for your effort, and I hope our perspective is understandable. :)

@LalithShiyam @Keyn34

Thank you very much for the insight.
I think having a separate parser script in our workflow would be the best way to go in that case :)

@LalithShiyam LalithShiyam added the wontfix This will not be worked on label Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants