neuropoly / data-management

Repo that deals with datalad aspects for internal use
4 stars 0 forks source link

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

Closed valosekj closed 4 months ago

valosekj commented 5 months ago

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 commented 4 months ago

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 commented 4 months ago

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 https://github.com/neuropoly/data-management/issues/282#issue-1995244880.

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 commented 4 months ago

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

I fixed participants.json and merged into master.