Fixes #611 Participants.tsv -> participants.tsv by MichaelStritt · Pull Request #749 · ExploreASL/ExploreASL

MichaelStritt · 2021-07-29T09:24:38Z

Linked issue

Check out #611

How to test

Run test dataset in BIDS format with old and new participants.tsv / Participants.tsv

MichaelStritt · 2021-07-29T10:27:28Z

@jan-petr: The way I wrote this in cb286d9 it basically works for that individual script, but that's not really how we should do this. Maybe we should rather revert cb286d9 and do this regexp check ...

    subjectNameOld = subjectName;
    if isempty(regexp(subjectNameOld,'sub-')==1)
        subjectName = ['sub-' subjectNameOld];
        fprintf('Change subject name: %s -> %s\n',subjectNameOld,subjectName);
    end

... at the beginning of the pipeline and use it to rename the overall dataset. Otherwise you will always suffer from subsequent faults like these ones ...

Printing csv-files with volume statistics...100%
Change subject name: 109 -> sub-109
Change subject name: 109 -> sub-109
Change subject name: 109 -> sub-109
Change subject name: 109 -> sub-109
Change subject name: 109 -> sub-109
Change subject name: 109 -> sub-109
Change subject name: 109 -> sub-109
050_GetVolumeStatistics was performed
Collecting motion metadata with motion statistics:100%
Saving motion plot to .\Siemens_2DEPI_noBsup_FLAIR_LoQ\Population\MotionASL\Overview_motion_pair-exclusion.jpg
Change subject name: 109 -> sub-109
060_GetMotionStatistics was performed
Loading & saving individual parameter files...100%
065_GetRegistrationStatistics was performed
participants.tsv (BIDS) detected, loading...
Loading AcquisitionTime:100%
Variable AcquisitionTime not included because all subjects were missing
Loading GM_vol:100%
Variable GM_vol not included because all subjects were missing
Loading WM_vol:100%
Variable WM_vol not included because all subjects were missing
Loading CSF_vol:100%
Variable CSF_vol not included because all subjects were missing
Loading GM_ICVRatio:100%
Variable GM_ICVRatio not included because all subjects were missing
Loading GMWM_ICVRatio:100%
Variable GMWM_ICVRatio not included because all subjects were missing
Loading WMH_vol:100%
Variable WMH_vol not included because all subjects were missing
Loading WMH_count:100%
Variable WMH_count not included because all subjects were missing
Loading MeanMotion:100%
Variable MeanMotion not included because all subjects were missing

MichaelStritt · 2021-07-29T13:48:26Z

Move this ...

    if isempty(regexp(subjectName ,'sub-')==1)
        subjectName = ['sub-' subjectName ];
        fprintf('Change subject name: %s -> %s\n',subjectNameOld,subjectName);
    end

... to the import (in a separate issue?).

jan-petr

Minor fixes needed. otherwise looks good.

jan-petr · 2021-07-29T20:12:47Z

Functions/xASL_init_LoadMetadata.m

-    PathTSV = fullfile(x.D.ROOT, 'Participants.tsv');
+    PathTSV = fullfile(x.D.ROOT, 'participants.tsv');
+    PathTSVold = fullfile(x.D.ROOT, 'Participants.tsv');
+    bParticipantsTSV = false;


to me, it is better to rename it if it detects the old one...

You're right. That's probably the more straight forward way to do this.
2b189c8 👍

thanks for the edit. Same as above - do we need the temp? Should we check for existence of both

jan-petr · 2021-07-29T20:13:19Z

Functions/xASL_init_LoadMetadata.m

+        bParticipantsTSV = true;
+    end
+
+    if bParticipantsTSV


especially since the 'participatns.tsv' is reported here.

2b189c8 👍

jan-petr · 2021-07-29T20:13:57Z

Modules/SubModule_Population/xASL_wrp_CreatePopulationTemplates.m

 %   bUpdateMetadata   - boolean specifying if we reload the metadata, e.g.
 %                       for potentially other defined cohorts etc in the
-%                       Participants.tsv. This can some time though.  Only relevant when computing multiple sets. 
+%                       participants.tsv. This can some time though.  Only relevant when computing multiple sets. 


"this can take some time" :D

b30ab8f 👍

MichaelStritt · 2021-07-30T07:44:13Z

Okay, minor problem... file names are not case sensitive on windows...

MichaelStritt · 2021-07-30T08:32:30Z

Test with existing (old) `Participants.tsv` within the BIDS root directory

It perfectly renamed, moved and filled it:

jan-petr

Almost there :)

Functions/xASL_bids_BIDS2Legacy.m

jan-petr · 2021-07-30T10:24:35Z

Functions/xASL_init_LoadMetadata.m

-    PathTSV = fullfile(x.D.ROOT, 'Participants.tsv');
+    PathTSV = fullfile(x.D.ROOT, 'participants.tsv');
+    PathTSVold = fullfile(x.D.ROOT, 'Participants.tsv');
+    bParticipantsTSV = false;


thanks for the edit. Same as above - do we need the temp? Should we check for existence of both

jan-petr

OK to merge after the 2 comments are added.

The previous code did not work because windows will complain if you move Participants.tsv to participants.tsv, since it regards both as the same file. The easiest fix is to rename it to another filename temporarily in between. Additionally we had the problem that you can not find the lowercase/uppercase differences using exist or xASL_exist, but xASL_adm_GetFileList uses regular expressions and worked nicely, so I implemented a fix with that. Makes the code a bit clunky though.

…participants.tsv

MichaelStritt added the bids Moving ExploreASL to BIDS compatibility label Jul 29, 2021

MichaelStritt requested a review from jan-petr July 29, 2021 09:24

MichaelStritt self-assigned this Jul 29, 2021

MichaelStritt linked an issue Jul 29, 2021 that may be closed by this pull request

Participants.tsv in BIDS format #611

Closed

jan-petr requested changes Jul 29, 2021

View reviewed changes

MichaelStritt force-pushed the bids-#611_participants branch from cb286d9 to b30ab8f Compare July 30, 2021 07:11

MichaelStritt requested a review from jan-petr July 30, 2021 08:34

jan-petr requested changes Jul 30, 2021

View reviewed changes

MichaelStritt requested a review from jan-petr July 30, 2021 10:44

jan-petr approved these changes Jul 30, 2021

View reviewed changes

MichaelStritt added 7 commits July 30, 2021 12:57

#611 Participants.tsv -> participants.tsv

67e66f7

#611 xASL_wrp_CreatePopulationTemplates: improve header

98c20a8

#611 xASL_init_LoadMetadata: rename Participants.tsv->lowercase

1b122db

#611 xASL_bids_BIDS2Legacy: rename participants file

fc8431a

#611 xASL_bids_BIDS2Legacy: minor fix for path definition

3ae7582

#611 Add descriptive comments for backwards compatibility feature of …

f179125

…participants.tsv

MichaelStritt force-pushed the bids-#611_participants branch from fe42f18 to f179125 Compare July 30, 2021 10:57

MichaelStritt merged commit f179125 into develop Jul 30, 2021

MichaelStritt deleted the bids-#611_participants branch July 30, 2021 10:58

Conversation

MichaelStritt commented Jul 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Linked issue

How to test

Uh oh!

MichaelStritt commented Jul 29, 2021

Uh oh!

MichaelStritt commented Jul 29, 2021

Uh oh!

jan-petr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MichaelStritt commented Jul 30, 2021

Uh oh!

MichaelStritt commented Jul 30, 2021

Test with existing (old) Participants.tsv within the BIDS root directory

Uh oh!

jan-petr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jan-petr left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

MichaelStritt commented Jul 29, 2021 •

edited

Loading

Test with existing (old) `Participants.tsv` within the BIDS root directory