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

dcm-zurich-lesions-20231115 - add T2w ax SC GT #295

Closed
valosekj opened this issue Feb 2, 2024 · 3 comments
Closed

dcm-zurich-lesions-20231115 - add T2w ax SC GT #295

valosekj opened this issue Feb 2, 2024 · 3 comments

Comments

@valosekj
Copy link
Member

valosekj commented Feb 2, 2024

I added SC segs for T2w ax images to the dcm-zurich-lesions-20231115 dataset in the jv/add_sc_seg branch.

SC segs were obtained using the SCIseg nnUNet 3D model, visually QCed, and corrected in 5 subjects. For details, use git log --stat.

@mguaypaq, could you please check the branch and merge? Thanks!

@mguaypaq
Copy link
Member

mguaypaq commented Feb 9, 2024

git annex get is fine. It's nice that you added "SpatialReference": "orig" to the json sidecars. I'm not sure about the naming scheme:

sub-01_acq-ax_T2w_label-SC_mask-manual.nii.gz

Specifically, mask-manual isn't BIDS-compatible (since mask should be a suffix, with no -manual added to it), and I don't see anything like it in our internal curation guide. Can we drop the -manual? Or maybe, put it as _desc-manual_mask?

Separately, I'm noticing that in the latest commit on master, I changed participants.tsv without updating the column descriptions in participants.json; I'll add another commit before merging this PR to fix this.

@valosekj
Copy link
Member Author

valosekj commented Feb 9, 2024

I'm not sure about the naming scheme:

sub-01_acq-ax_T2w_label-SC_mask-manual.nii.gz

Specifically, mask-manual isn't BIDS-compatible (since mask should be a suffix, with no -manual added to it), and I don't see anything like it in our internal curation guide. Can we drop the -manual? Or maybe, put it as _desc-manual_mask?

You are right, the current suffix does not follow our convention. I chose such a suffix to be consistent with the dcm-zurich-lesions dataset (I am combining both datasets for model training-- different suffixes would complicate the dataset combining.). I would vote for merging now and fixing suffixes for both dcm-zurich-lesions and dcm-zurich-lesions-20231115 as part of #282 (comment).

Separately, I'm noticing that in the latest commit on master, I changed participants.tsv without updating the column descriptions in participants.json; I'll add another commit before merging this PR to fix this.

Thank you!

@mguaypaq
Copy link
Member

Alright, since you have a specific reason for the naming scheme, that's ok.

I fixed participants.json and merged into master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants