DCAN-Labs / dcan_bold_processing

functional connectivity preprocessing for resting state fMRI outputs of the dcan-fmri-pipelines
BSD 3-Clause "New" or "Revised" License
7 stars 7 forks source link

regex changes, fixes for abcd-hcp-pipeline + nhp-abcd-bids-pipeline #15

Closed madisoth closed 3 years ago

madisoth commented 3 years ago
arueter1 commented 3 years ago

@madisoth Can you make the 1 little change that Kathy suggested and then I will approve it and merge. :) Thanks!

madisoth commented 3 years ago

@kathy-snider I agree with your suggestion, I will get it added! The one drawback is that it's going to take extra work to accommodate both run indices of arbitrary length and task names ending in digits; e.g. does "ses-mySes-task-T0001.dtseries.nii" mean:

task=T, run=0001 task=T00 run=01 task=T0001, run=n/a
?

so for now I'll just add your suggested fix, and disclaimer that task names ending in digits won't be supported.

kathy-snider commented 3 years ago

This would only affect the BIDS version, i.e., non-legacy. So you could have task-rest_run-1 or task-rest_run-000001 and the "+" would get all of the digits whereas the {2,} only gets the digits on the second one (run-000001), but would not match the first one (run-1).


From: Thomas Madison @.***> Sent: Wednesday, August 4, 2021 11:03 AM To: DCAN-Labs/dcan_bold_processing Cc: Kathy Snider; Mention Subject: [EXTERNAL] Re: [DCAN-Labs/dcan_bold_processing] regex changes, fixes for abcd-hcp-pipeline + nhp-abcd-bids-pipeline (#15)

@kathy-sniderhttps://github.com/kathy-snider I agree with your suggestion, I will get it added! The one drawback is that it's going to take extra work to accommodate both run indices of arbitrary length and task names ending in digits; e.g. does "ses-mySes-task-T0001.dtseries.nii" mean:

task=T, run=0001 task=T00 run=01 task=T0001, run=n/a ?

so for now I'll just add your suggested fix, and disclaimer that task names ending in digits won't be supported unless you have two-digit run indices and use the --legacy-tasknames flag.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/DCAN-Labs/dcan_bold_processing/pull/15#issuecomment-892860839, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AJAQP4HJM2PA5ZQE6H4KBDLT3F6GJANCNFSM5BGQUJGQ. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email.

madisoth commented 3 years ago

Ahh that makes sense now that you explained it, I'm getting confused by my own code. Merging now.