ENH: Finish qMRI file-level rules #1186

effigies · 2021-02-11T19:09:06Z

Partner PR to bids-standard/bids-specification#728. If that goes in without modification, this should be able to as well.

codecov · 2021-02-11T19:15:31Z

Codecov Report

Merging #1186 (b98eb6a) into master (c1ad282) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1186      +/-   ##
==========================================
+ Coverage   79.07%   79.09%   +0.02%     
==========================================
  Files          78       78              
  Lines        2619     2622       +3     
  Branches      598      598              
==========================================
+ Hits         2071     2074       +3     
  Misses        407      407              
  Partials      141      141

Impacted Files	Coverage Δ
bids-validator/utils/type.js	`98.94% <100.00%> (+0.03%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c1ad282...b98eb6a. Read the comment docs.

tsalo

I don't know how important that extra ? is, but it's also missing from the pattern for _TB1DAM.

bids-validator/bids_validator/rules/file_level_rules.json

Co-authored-by: Taylor Salo <tsalo006@fiu.edu>

effigies · 2021-02-13T01:15:30Z

(?:...) just means that the group won't be captured, e.g., you can't use a back-reference to say "find the same chunk again". (...)? means the group can be present or absent.

sappelhoff

I tested this PR on the qMRI examples PR, and there are some errors left:

Validating dataset qmri_mp2rage/ bids-validator@1.6.2-dev.0

    1: [WARN] The Authors field of dataset_description.json should contain an array of fields - with one author per field. This was triggered because there are no authors, which will make DOI registration from dataset metadata impossible. (code: 113 - NO_AUTHORS)

    Please visit https://neurostars.org/search?q=NO_AUTHORS for existing conversations about this issue.


    Summary:             Available Tasks:        Available Modalities: 
    9 Files, 436B                                MP2RAGE               
    1 - Subject                                  defacemask            
    1 - Session                                                        


    If you have any questions, please post on https://neurostars.org/tags/bids.

Validating dataset qmri_mp2rageme/
bids-validator@1.6.2-dev.0

    1: [ERR] Files with such naming scheme are not part of BIDS specification. This error is most commonly caused by typos in file names that make them not BIDS compatible. Please consult the specification and make sure your files are named correctly. If this is not a file naming issue (for example when including files not yet covered by the BIDS specification) you should include a ".bidsignore" file in your dataset (see https://github.com/bids-standard/bids-validator#bidsignore for details). Please note that derived (processed) data should be placed in /derivatives folder and source data (such as DICOMS or behavioural logs in proprietary formats) should be placed in the /sourcedata folder. (code: 1 - NOT_INCLUDED)
            ./sub-1/fmap/sub-1_TB1map.json
                    Evidence: sub-1_TB1map.json
            ./sub-1/fmap/sub-1_TB1map.nii
                    Evidence: sub-1_TB1map.nii

    Please visit https://neurostars.org/search?q=NOT_INCLUDED for existing conversations about this issue.

    1: [WARN] The Authors field of dataset_description.json should contain an array of fields - with one author per field. This was triggered because there are no authors, which will make DOI registration from dataset metadata impossible. (code: 113 - NO_AUTHORS)

    Please visit https://neurostars.org/search?q=NO_AUTHORS for existing conversations about this issue.


    Summary:                Available Tasks:        Available Modalities: 
    19 Files, 1.55KB                                MP2RAGE               
    1 - Subject                                                           
    1 - Session                                                           


    If you have any questions, please post on https://neurostars.org/tags/bids.

Validating dataset qmri_mpm/
bids-validator@1.6.2-dev.0

    1: [ERR] Files with such naming scheme are not part of BIDS specification. This error is most commonly caused by typos in file names that make them not BIDS compatible. Please consult the specification and make sure your files are named correctly. If this is not a file naming issue (for example when including files not yet covered by the BIDS specification) you should include a ".bidsignore" file in your dataset (see https://github.com/bids-standard/bids-validator#bidsignore for details). Please note that derived (processed) data should be placed in /derivatives folder and source data (such as DICOMS or behavioural logs in proprietary formats) should be placed in the /sourcedata folder. (code: 1 - NOT_INCLUDED)
            ./sub-01/fmap/sub-01_acq-bodyMTw_RB1COR.json
                    Evidence: sub-01_acq-bodyMTw_RB1COR.json
            ./sub-01/fmap/sub-01_acq-bodyMTw_RB1COR.nii
                    Evidence: sub-01_acq-bodyMTw_RB1COR.nii
            ./sub-01/fmap/sub-01_acq-bodyPDw_RB1COR.json
                    Evidence: sub-01_acq-bodyPDw_RB1COR.json
            ./sub-01/fmap/sub-01_acq-bodyPDw_RB1COR.nii
                    Evidence: sub-01_acq-bodyPDw_RB1COR.nii
            ./sub-01/fmap/sub-01_acq-bodyT1w_RB1COR.json
                    Evidence: sub-01_acq-bodyT1w_RB1COR.json
            ./sub-01/fmap/sub-01_acq-bodyT1w_RB1COR.nii
                    Evidence: sub-01_acq-bodyT1w_RB1COR.nii
            ./sub-01/fmap/sub-01_acq-headMTw_RB1COR.json
                    Evidence: sub-01_acq-headMTw_RB1COR.json
            ./sub-01/fmap/sub-01_acq-headMTw_RB1COR.nii
                    Evidence: sub-01_acq-headMTw_RB1COR.nii
            ./sub-01/fmap/sub-01_acq-headPDw_RB1COR.json
                    Evidence: sub-01_acq-headPDw_RB1COR.json
            ./sub-01/fmap/sub-01_acq-headPDw_RB1COR.nii
                    Evidence: sub-01_acq-headPDw_RB1COR.nii
            ... and 2 more files having this issue (Use --verbose to see them all).

    Please visit https://neurostars.org/search?q=NOT_INCLUDED for existing conversations about this issue.

    1: [WARN] 'TotalReadoutTime' is greater than 10 are you sure it's expressed in seconds? (code: 5 - TOTAL_READOUT_TIME_GREATER_THAN)
            ./sub-01/fmap/sub-01_echo-1_flip-01_TB1EPI.json
            ./sub-01/fmap/sub-01_echo-1_flip-02_TB1EPI.json
            ./sub-01/fmap/sub-01_echo-1_flip-03_TB1EPI.json
            ./sub-01/fmap/sub-01_echo-1_flip-04_TB1EPI.json
            ./sub-01/fmap/sub-01_echo-1_flip-05_TB1EPI.json
            ./sub-01/fmap/sub-01_echo-1_flip-06_TB1EPI.json
            ./sub-01/fmap/sub-01_echo-1_flip-07_TB1EPI.json
            ./sub-01/fmap/sub-01_echo-1_flip-08_TB1EPI.json
            ./sub-01/fmap/sub-01_echo-1_flip-09_TB1EPI.json
            ./sub-01/fmap/sub-01_echo-1_flip-10_TB1EPI.json
            ... and 12 more files having this issue (Use --verbose to see them all).

    Please visit https://neurostars.org/search?q=TOTAL_READOUT_TIME_GREATER_THAN for existing conversations about this issue.

    2: [WARN] The Authors field of dataset_description.json should contain an array of fields - with one author per field. This was triggered based on the presence of only one author field. Please ignore if all contributors are already properly listed. (code: 102 - TOO_FEW_AUTHORS)

    Please visit https://neurostars.org/search?q=TOO_FEW_AUTHORS for existing conversations about this issue.


    Summary:                  Available Tasks:        Available Modalities: 
    108 Files, 54.84KB                                MPM                   
    1 - Subject                                       TB1EPI                
    1 - Session                                       fieldmap              


    If you have any questions, please post on https://neurostars.org/tags/bids.

Validating dataset qmri_mtsat/
bids-validator@1.6.2-dev.0

    1: [WARN] The Authors field of dataset_description.json should contain an array of fields - with one author per field. This was triggered based on the presence of only one author field. Please ignore if all contributors are already properly listed. (code: 102 - TOO_FEW_AUTHORS)

    Please visit https://neurostars.org/search?q=TOO_FEW_AUTHORS for existing conversations about this issue.


    Summary:             Available Tasks:        Available Modalities: 
    8 Files, 532B                                MTS                   
    1 - Subject                                                        
    1 - Session                                                        


    If you have any questions, please post on https://neurostars.org/tags/bids.

Validating dataset qmri_qsm/
bids-validator@1.6.2-dev.0

    1: [WARN] The Authors field of dataset_description.json should contain an array of fields - with one author per field. This was triggered based on the presence of only one author field. Please ignore if all contributors are already properly listed. (code: 102 - TOO_FEW_AUTHORS)

    Please visit https://neurostars.org/search?q=TOO_FEW_AUTHORS for existing conversations about this issue.


    Summary:             Available Tasks:        Available Modalities: 
    4 Files, 864B                                Chimap                
    1 - Subject                                                        
    1 - Session                                                        


    If you have any questions, please post on https://neurostars.org/tags/bids.

Validating dataset qmri_sa2rage/
bids-validator@1.6.2-dev.0

    1: [ERR] Files with such naming scheme are not part of BIDS specification. This error is most commonly caused by typos in file names that make them not BIDS compatible. Please consult the specification and make sure your files are named correctly. If this is not a file naming issue (for example when including files not yet covered by the BIDS specification) you should include a ".bidsignore" file in your dataset (see https://github.com/bids-standard/bids-validator#bidsignore for details). Please note that derived (processed) data should be placed in /derivatives folder and source data (such as DICOMS or behavioural logs in proprietary formats) should be placed in the /sourcedata folder. (code: 1 - NOT_INCLUDED)
            ./sub-01/fmap/sub-01_acq-td1_TB1SRGE.json
                    Evidence: sub-01_acq-td1_TB1SRGE.json
            ./sub-01/fmap/sub-01_acq-td1_TB1SRGE.nii.gz
                    Evidence: sub-01_acq-td1_TB1SRGE.nii.gz
            ./sub-01/fmap/sub-01_acq-td2_TB1SRGE.json
                    Evidence: sub-01_acq-td2_TB1SRGE.json
            ./sub-01/fmap/sub-01_acq-td2_TB1SRGE.nii.gz
                    Evidence: sub-01_acq-td2_TB1SRGE.nii.gz

    Please visit https://neurostars.org/search?q=NOT_INCLUDED for existing conversations about this issue.

    1: [WARN] The Authors field of dataset_description.json should contain an array of fields - with one author per field. This was triggered because there are no authors, which will make DOI registration from dataset metadata impossible. (code: 113 - NO_AUTHORS)

    Please visit https://neurostars.org/search?q=NO_AUTHORS for existing conversations about this issue.


    Summary:                  Available Tasks:        Available Modalities: 
    10 Files, 216.04KB                                MP2RAGE               
    1 - Subject                                                             
    1 - Session                                                             


    If you have any questions, please post on https://neurostars.org/tags/bids.

Validating dataset qmri_vfa/
bids-validator@1.6.2-dev.0

    1: [ERR] Files with such naming scheme are not part of BIDS specification. This error is most commonly caused by typos in file names that make them not BIDS compatible. Please consult the specification and make sure your files are named correctly. If this is not a file naming issue (for example when including files not yet covered by the BIDS specification) you should include a ".bidsignore" file in your dataset (see https://github.com/bids-standard/bids-validator#bidsignore for details). Please note that derived (processed) data should be placed in /derivatives folder and source data (such as DICOMS or behavioural logs in proprietary formats) should be placed in the /sourcedata folder. (code: 1 - NOT_INCLUDED)
            ./sub-01/fmap/sub-01_TB1map.json
                    Evidence: sub-01_TB1map.json
            ./sub-01/fmap/sub-01_TB1map.nii.gz
                    Evidence: sub-01_TB1map.nii.gz

    Please visit https://neurostars.org/search?q=NOT_INCLUDED for existing conversations about this issue.

    1: [WARN] The Authors field of dataset_description.json should contain an array of fields - with one author per field. This was triggered based on the presence of only one author field. Please ignore if all contributors are already properly listed. (code: 102 - TOO_FEW_AUTHORS)

    Please visit https://neurostars.org/search?q=TOO_FEW_AUTHORS for existing conversations about this issue.


    Summary:             Available Tasks:        Available Modalities: 
    8 Files, 865B                                VFA                   
    1 - Subject                                                        
    1 - Session                                                        


    If you have any questions, please post on https://neurostars.org/tags/bids.

In particular:

--> some qmri datasets are passing now 🎉

sappelhoff · 2021-02-13T08:29:09Z

f714d63 fixes the errors reported above, except for:

sappelhoff · 2021-02-13T09:03:01Z

6058d9e fixes the remaining issue. The auto style fix has obscured the diff, but this was the essential change:

- ?_flip-[0-9]+?_inv-[0-9]+?
+ ?(?:_flip-[0-9]+)?(?:_inv-[0-9]+)?

In case this fix was wrong, we can revert it ... and fix the dataset instead.

In case this fix was fine, we can merge this and then we're almost done (only warnings of two kinds remaining for the qMRI examples). @tsalo @effigies

tsalo · 2021-02-13T16:27:33Z

Thanks @sappelhoff! Apologies if this came up elsewhere, but are we sure that the examples are right? TB1SRGE requires flip and inv in the schema right now, and the File collections page says that flip and inv are required as well.

sappelhoff · 2021-02-13T16:59:14Z

but are we sure that the examples are right?

Not sure 😞

sappelhoff · 2021-02-15T15:46:53Z

but are we sure that the examples are right? TB1SRGE requires flip and inv in the schema right now, and the File collections page says that flip and inv are required as well.

@agahkarakuzu @Gilles86 can you shed some light on this, please?

effigies · 2021-02-15T16:07:23Z

I added flip/inv to the example dataset.

agahkarakuzu · 2021-02-15T16:11:19Z

@sappelhoff I am also working on the example datasets on my fork (https://github.com/agahkarakuzu/BIDS-examples/tree/qmri), but had some issues with syncing changes with the validator.

As for TB1SRGE, flip and inv are required entities. Maybe I can go ahead and push all the examples first, then deal with the validator later?

sappelhoff · 2021-02-15T16:34:04Z

I added flip/inv to the example dataset.

Thanks @effigies - and as @agahkarakuzu mentioned right afterwards, this was good:

As for TB1SRGE, flip and inv are required entities.

Now we just need to:

revert my commit that made flip and inv optional
and then merge this PR
check if the examples are passing

Perhaps we can merge @effigies's PR into the https://github.com/bids-standard/bids-examples/tree/bep001_qmri branch first, instead of master. Then @agahkarakuzu can put his changes on top of that ... and then finally, we merge that branch into master.

WDYT?

This reverts commit 6058d9e.

effigies · 2021-02-15T16:38:28Z

Stefan, please feel free to do what you think necessary. I'll make you an admin on my forks of validator and examples.

effigies · 2021-02-15T16:41:30Z

bids-validator/utils/type.js

+            conditionalMatch(fmapTB1EPI, path))) ||
+        conditionalMatch(fmaprf, path) ||
+        conditionalMatch(fmapTB1SRGE, path) ||
+        conditionalMatch(fmapparametric, path)


Just a quick note that if this logic doesn't work, the idea is just find all files that match isFieldMap and aren't JSON. The original parenthesis pattern could be restored.

* ENH: Finish qMRI file-level rules * TB1RMF -> TB1RFM * Apply suggestions from code review Co-authored-by: Taylor Salo <tsalo006@fiu.edu> * add regexps to typs.js * make flip and inv optional for TB1SRGE * Revert "make flip and inv optional for TB1SRGE" This reverts commit 6058d9e. Co-authored-by: Taylor Salo <tsalo006@fiu.edu> Co-authored-by: Stefan Appelhoff <stefan.appelhoff@mailbox.org>

ENH: Finish qMRI file-level rules

a49a3f0

TB1RMF -> TB1RFM

6988a2b

tsalo requested changes Feb 13, 2021

View reviewed changes

Apply suggestions from code review

73ed1a6

Co-authored-by: Taylor Salo <tsalo006@fiu.edu>

sappelhoff reviewed Feb 13, 2021

View reviewed changes

add regexps to typs.js

f714d63

make flip and inv optional for TB1SRGE

6058d9e

sappelhoff mentioned this pull request Feb 13, 2021

ENH: Add qMRI example datasets bids-standard/bids-examples#245

Merged

Revert "make flip and inv optional for TB1SRGE"

b98eb6a

This reverts commit 6058d9e.

sappelhoff merged commit a02ccd8 into bids-standard:master Feb 15, 2021

effigies commented Feb 15, 2021

View reviewed changes

effigies mentioned this pull request Feb 15, 2021

ENH: Make fmap rules in type.js more consistent #1191

Merged

effigies deleted the qmri/fmaps branch February 15, 2021 18:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: Finish qMRI file-level rules #1186

ENH: Finish qMRI file-level rules #1186

effigies commented Feb 11, 2021

codecov bot commented Feb 11, 2021 •

edited

Loading

tsalo left a comment

effigies commented Feb 13, 2021

sappelhoff left a comment

sappelhoff commented Feb 13, 2021

sappelhoff commented Feb 13, 2021

tsalo commented Feb 13, 2021

sappelhoff commented Feb 13, 2021

sappelhoff commented Feb 15, 2021

effigies commented Feb 15, 2021

agahkarakuzu commented Feb 15, 2021

sappelhoff commented Feb 15, 2021

effigies commented Feb 15, 2021

effigies Feb 15, 2021

ENH: Finish qMRI file-level rules #1186

ENH: Finish qMRI file-level rules #1186

Conversation

effigies commented Feb 11, 2021

codecov bot commented Feb 11, 2021 • edited Loading

Codecov Report

tsalo left a comment

Choose a reason for hiding this comment

effigies commented Feb 13, 2021

sappelhoff left a comment

Choose a reason for hiding this comment

sappelhoff commented Feb 13, 2021

sappelhoff commented Feb 13, 2021

tsalo commented Feb 13, 2021

sappelhoff commented Feb 13, 2021

sappelhoff commented Feb 15, 2021

effigies commented Feb 15, 2021

agahkarakuzu commented Feb 15, 2021

sappelhoff commented Feb 15, 2021

effigies commented Feb 15, 2021

effigies Feb 15, 2021

Choose a reason for hiding this comment

codecov bot commented Feb 11, 2021 •

edited

Loading