-
Notifications
You must be signed in to change notification settings - Fork 297
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
[ENH] Store BOLD reference images #1306
Conversation
Just want to say: I think the renaming process should be all-or-nothing. Once it starts, we should finish before releasing. Do we want to make a release before starting to merge PRs like this? |
Yesss |
.circleci/ds005_outputs.txt
Outdated
fmriprep/sub-01/func/sub-01_task-mixedgamblestask_run-01_space-fsaverage5.L.func.gii | ||
fmriprep/sub-01/func/sub-01_task-mixedgamblestask_run-01_space-fsaverage5.R.func.gii | ||
fmriprep/sub-01/func/sub-01_task-mixedgamblestask_run-01_space-MNI152NLin2009cAsym_brainmask.nii.gz | ||
fmriprep/sub-01/func/sub-01_task-mixedgamblestask_run-01_space-MNI152NLin2009cAsym_desc-3dref_bold.nii.gz |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this qualifies as a bold.nii.gz
, as it's not a 4D time series. We didn't try to get a ref
suffix (or perhaps boldref
, since there is no longer a bold
entry) into RC1, but we can still use it as long as there isn't a more appropriate suffix in the spec.
Pinging BIDS-meister @chrisfilo for a second opinion, though.
You're right - I would use a new suffix and be ready to change it some time
in the future when this is standardized.
…On Thu, Oct 4, 2018, 5:56 AM Chris Markiewicz ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In .circleci/ds005_outputs.txt
<#1306 (comment)>:
> -fmriprep/sub-01/func/sub-01_task-mixedgamblestask_run-02_bold_space-MNI152NLin2009cAsym_brainmask.nii.gz
-fmriprep/sub-01/func/sub-01_task-mixedgamblestask_run-02_bold_space-MNI152NLin2009cAsym_preproc.nii.gz
-fmriprep/sub-01/func/sub-01_task-mixedgamblestask_run-02_bold_space-MNI152NLin2009cAsym_variant-smoothAROMAnonaggr_preproc.nii.gz
-fmriprep/sub-01/func/sub-01_task-mixedgamblestask_run-02_bold_space-T1w_brainmask.nii.gz
-fmriprep/sub-01/func/sub-01_task-mixedgamblestask_run-02_bold_space-T1w_label-aparcaseg_roi.nii.gz
-fmriprep/sub-01/func/sub-01_task-mixedgamblestask_run-02_bold_space-T1w_label-aseg_roi.nii.gz
-fmriprep/sub-01/func/sub-01_task-mixedgamblestask_run-02_bold_space-T1w_preproc.nii.gz
+fmriprep/sub-01/func/sub-01_task-mixedgamblestask_run-01_AROMAnoiseICs.csv
+fmriprep/sub-01/func/sub-01_task-mixedgamblestask_run-01_confounds.tsv
+fmriprep/sub-01/func/sub-01_task-mixedgamblestask_run-01_MELODICmix.tsv
+fmriprep/sub-01/func/sub-01_task-mixedgamblestask_run-01_space-cifti_variant-space1_preproc.dtseries.json
+fmriprep/sub-01/func/sub-01_task-mixedgamblestask_run-01_space-cifti_variant-space1_preproc.dtseries.nii
+fmriprep/sub-01/func/sub-01_task-mixedgamblestask_run-01_space-fsaverage5.L.func.gii
+fmriprep/sub-01/func/sub-01_task-mixedgamblestask_run-01_space-fsaverage5.R.func.gii
+fmriprep/sub-01/func/sub-01_task-mixedgamblestask_run-01_space-MNI152NLin2009cAsym_brainmask.nii.gz
+fmriprep/sub-01/func/sub-01_task-mixedgamblestask_run-01_space-MNI152NLin2009cAsym_desc-3dref_bold.nii.gz
I don't think this qualifies as a bold.nii.gz, as it's not a 4D time
series. We didn't try to get a ref suffix (or perhaps boldref, since
there is no longer a bold entry) into RC1, but we can still use it as
long as there isn't a more appropriate suffix in the spec.
Pinging BIDS-meister @chrisfilo <https://github.com/chrisfilo> for a
second opinion, though.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1306 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAOkp487rz40_1t2nFZ5JMMaBOjm7D1cks5uhgVxgaJpZM4XHLmv>
.
|
thumbs up? |
Sure. Let's move pretty quickly on the rest of the renamings, then. I listed the anat ones. I'll check for any functional ones that aren't covered by that in the morning. |
In #1306, ``DataSink``s were updated and the modality entity was not writen on the name anymore. However, I missed to update the reports configuration to contemplate the change. This hotfix addresses the problem.
Changes proposed in this pull request
DerivativesDataSink
not to insert the_<bids_type>_
in the middle of every derivative name.Documentation that should be reviewed