Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[ENH] Bep 009: Positron Emission Tomography #1088

Merged
merged 55 commits into from
Apr 14, 2021
Merged

[ENH] Bep 009: Positron Emission Tomography #1088

merged 55 commits into from
Apr 14, 2021

Conversation

melanieganz
Copy link

This PR incorporates BEP 009 - Positron Emission Tomography into the validator.

We have rebased bep009 with master before including all our BEP 009 changes into the bep009 branch and there are no direct conflicts. Hence, we would like to open this up to community comment.

Moderators: @melanieganz @mnoergaard

@hoechenberger hoechenberger removed their request for review October 1, 2020 11:07
@hoechenberger
Copy link
Collaborator

Sorry, I won't have time to review such a massive PR… unassigned myself.

@nellh
Copy link
Member

nellh commented Oct 1, 2020

Looks like this PR is accidentally reverting a number of changes from master in f7d4077 and that will need to be fixed before it can really be reviewed.

mnoergaard and others added 12 commits February 25, 2021 16:44
@mnoergaard
Merge pull request #2 from bids-standard/master
c47e11d 
Update mnorgaard/bids-validator according to main branch
@mnoergaard
Added PET specs to validator
5e3c762
@mnoergaard
Update pet.json
496c7e3
@mnoergaard
Add blood schema to json.js
3e8c867
@mnoergaard
Update tsv.js to include blood data for pet
2af6fc3
@mnoergaard
Update file_level_rules.json to include pet and blood
666059d
@mnoergaard
Update session_level_rules and non-custom_columns to include PET data
a94b87d
@mnoergaard
Update session_level_rules.json
4d9e2db
@mnoergaard
Updated top_level_rules.json and non_custom_columns for PET
221adfd
@mnoergaard
Update type.js
0dc7793
@mnoergaard
Updated schemas and file_levels_rules to accomodate pet data
7c5d136
@effigies
Squashed commit of the following:
841780c 
commit 9529274
Author: Ross Blair <rosswilsonblair@gmail.com>
Date:   Mon Oct 19 12:11:27 2020 -0500

    remove isCont and add back in behavioralData in type.js

commit 4e36b4f
Merge: e32ff56 556e86d
Author: Ross Blair <rosswilsonblair@gmail.com>
Date:   Mon Oct 19 11:07:30 2020 -0500

    Merge branch 'master' of github.com:bids-standard/bids-validator into bep_009_rev

commit e32ff56
Author: Ross Blair <rosswilsonblair@gmail.com>
Date:   Mon Oct 19 10:45:36 2020 -0500

    Revert "Reset validator to original (martin branch) after failed merge"

    This reverts commit f7d4077.

commit a888d48
Merge: f7effef f7d4077
Author: melanieganz <melanieganz@users.noreply.github.com>
Date:   Thu Oct 1 12:50:38 2020 +0200

    Merge pull request #1087 from mnoergaard/martin

    [ENH] Update BEP009 with newest version of validator #1085

commit f7d4077
Author: mnoergaard <martin.noergaard@nru.dk>
Date:   Thu Oct 1 11:34:11 2020 +0200

    Reset validator to original (martin branch) after failed merge

commit 67dc1e4
Merge: 34debcf f7effef
Author: mnoergaard <martin.noergaard@nru.dk>
Date:   Thu Oct 1 11:10:48 2020 +0200

    Merge branch 'bep_009' into martin

commit f7effef
Merge: 73580bf 25daeb5
Author: melanieganz <melanieganz@users.noreply.github.com>
Date:   Thu Oct 1 10:11:46 2020 +0200

    Merge pull request #1084 from bids-standard/master

    [MISC] Update of BEP009 validator branch with all master changes before pull request

commit 34debcf
Author: mnoergaard <martin.noergaard@nru.dk>
Date:   Tue Sep 8 21:22:38 2020 +0200

    Updated pet schema, and included a blood schema for pet to accomodate blood data

commit 235657b
Author: mnoergaard <martin.noergaard@nru.dk>
Date:   Fri Aug 21 13:49:25 2020 +0200

    Updated schema pet.json to accomodate FDG scans

commit 4643f51
Author: mnoergaard <martin.noergaard@nru.dk>
Date:   Wed Aug 19 16:24:51 2020 +0200

    Delete error.log

commit 4711721
Author: mnoergaard <martin.noergaard@nru.dk>
Date:   Wed Aug 5 14:03:25 2020 +0200

    Minor fixes of the validator (PET) to align with the newest standard

commit 9c89bcb
Author: mnoergaard <martin.noergaard@nru.dk>
Date:   Tue Aug 4 14:34:00 2020 +0200

    Updated pet schema according to newest standard (04082020)

commit c6f6b24
Author: Niveauet <emil@niveauet.dk>
Date:   Tue Jun 16 11:16:19 2020 +0200

    Edits according to the Google doc

commit a62fc4d
Author: Niveauet <hej@emildr.dk>
Date:   Sat Jun 6 02:32:52 2020 +0200

    Last update before hand-in

commit 9722163
Author: Niveauet <hej@emildr.dk>
Date:   Wed May 27 15:02:02 2020 +0200

    Changed TSV file name last place

commit eb23b96
Author: Niveauet <hej@emildr.dk>
Date:   Tue May 26 19:55:33 2020 +0200

    Additional rules

commit 19c478f
Author: Niveauet <emil@niveauet.dk>
Date:   Wed May 20 14:23:36 2020 +0200

    Changes according to document

commit 53844b5
Merge: 845970c ea23f3f
Author: Niveauet <hej@emildr.dk>
Date:   Sat May 16 15:55:08 2020 +0200

    Merge branch 'master' of https://github.com/Niveauet/bids-validator

commit 845970c
Author: Niveauet <hej@emildr.dk>
Date:   Fri Mar 6 15:25:31 2020 +0100

    Validation of PET data

commit ea23f3f
Author: Niveauet <hej@emildr.dk>
Date:   Thu May 14 11:25:51 2020 +0200

    Changes according to the document

commit e9396ad
Author: Niveauet <hej@emildr.dk>
Date:   Thu May 7 17:06:41 2020 +0200

    Fixed columns for TSV file

commit ffd42fe
Author: Niveauet <hej@emildr.dk>
Date:   Thu May 7 15:38:07 2020 +0200

    Now requires a tsv file for pet blood data.

commit 6bbac9c
Author: Niveauet <hej@emildr.dk>
Date:   Wed May 6 19:58:23 2020 +0200

    Split file up in 2 (still need blood data data side car, maybe tsv?)

commit afca8c7
Author: Niveauet <hej@emildr.dk>
Date:   Tue May 5 13:56:37 2020 +0200

    Flattened JSON structure

commit 98eb6bd
Author: Niveauet <hej@emildr.dk>
Date:   Thu Apr 30 18:58:24 2020 +0200

    Conditional requirement of field if another one is true

commit 62d1e50
Author: Niveauet <hej@emildr.dk>
Date:   Thu Apr 30 17:01:47 2020 +0200

    Rules for time and min max on numbers

commit 7e40e83
Author: Niveauet <emil@niveauet.dk>
Date:   Thu Apr 30 16:03:54 2020 +0200

    Fields, requirements and dependencies

commit abf69ec
Author: Niveauet <hej@emildr.dk>
Date:   Tue Apr 28 00:18:43 2020 +0200

    More work on the validator

commit 7da07bf
Author: Niveauet <hej@emildr.dk>
Date:   Fri Apr 17 15:03:53 2020 +0200

    Added fields to PET json schema (WIP)

commit 536feac
Author: Niveauet <hej@emildr.dk>
Date:   Fri Mar 20 16:49:07 2020 +0100

    Added PET data to be verified along with the JSON

commit 73756b4
Author: Niveauet <hej@emildr.dk>
Date:   Fri Mar 20 16:03:51 2020 +0100

    Created regex rules for pet scan data files

commit acb530f
Author: Niveauet <hej@emildr.dk>
Date:   Fri Mar 20 15:23:22 2020 +0100

    Allow PET folder to be the parent of a nifty file

commit e688334
Author: Niveauet <hej@emildr.dk>
Date:   Fri Mar 6 15:25:31 2020 +0100

    Prepared for PET scan data
@effigies
Copy link
Collaborator

Resolved conflicts and squashed into master. There's a kind of open question about reverting #1122 (#1124), so I commented the part that is affected by that decision.

@codecov
Copy link

codecov bot commented Feb 26, 2021 •
edited
Loading

Codecov Report

Merging #1088 (485f6ae) into master (e7e0860) will increase coverage by 0.83%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1088      +/-   ##
==========================================
+ Coverage   82.30%   83.14%   +0.83%     
==========================================
  Files          78       78              
  Lines        2634     2682      +48     
  Branches      599      607       +8     
==========================================
+ Hits         2168     2230      +62     
+ Misses        377      365      -12     
+ Partials       89       87       -2
Impacted Files Coverage Δ
bids-validator/utils/modalities.js 100.00% <ø> (ø)
bids-validator/utils/type.js 99.00% <100.00%> (+0.06%) ⬆️
bids-validator/validators/json/json.js 100.00% <100.00%> (ø)
bids-validator/validators/tsv/tsv.js 96.85% <100.00%> (+0.05%) ⬆️
...ids-validator/validators/tsv/validateTsvColumns.js 55.11% <100.00%> (+17.75%) ⬆️
bids-validator/utils/issues/index.js 80.00% <0.00%> (-1.34%) ⬇️
bids-validator/validators/json/load.js 95.00% <0.00%> (+20.00%) ⬆️
bids-validator/utils/json.js 100.00% <0.00%> (+68.75%) ⬆️

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 e7e0860...485f6ae. Read the comment docs.

sappelhoff and others added 8 commits April 13, 2021 14:20
@sappelhoff @mnoergaard
fix: *Unit -> *Units ... in line with spec bids-standard/bids-specifi…
866049d 
…cation#773

Co-authored-by: Martin Norgaard <martin.noergaard@nru.dk>
@sappelhoff @mnoergaard
fix: typo in pet.json
7523797 
Co-authored-by: Martin Norgaard <martin.noergaard@nru.dk>
@sappelhoff @mnoergaard
misc fixes to bring validator in line with spec
82e56fc 
Co-authored-by: Martin Norgaard <martin.noergaard@nru.dk>
@sappelhoff @mnoergaard
fix: remove cruft -> some blood info no longer in pet.json
d9f8e0e 
see: https://github.com/bids-standard/bids-validator/pull/1088/files#r612415244

Co-authored-by: Martin Norgaard <martin.noergaard@nru.dk>
@rwblair
Merge branch 'master' of github.com:bids-standard/bids-validator into…
596bb37 
… bep_009
@rwblair
do a global replace on CRLF to LF and CR to LF in tsvParse
e59124e
@rwblair
Merge branch 'master' of github.com:bids-standard/bids-validator into…
e3d06f6 
… bep_009
@rwblair
fix json for pet example data, add relative path field where missing …
485f6ae 
…in tsv tests
@rwblair rwblair merged commit 161e88a into master Apr 14, 2021
BEP009: Positron Emission Tomography automation moved this from In progress to Done Apr 14, 2021
@effigies
Copy link
Collaborator

🎉 Nice work, all!

@sappelhoff sappelhoff deleted the bep_009 branch June 9, 2024 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tsalo tsalo tsalo left review comments

@mnoergaard mnoergaard mnoergaard left review comments

@rwblair rwblair rwblair left review comments

@david-nishi david-nishi david-nishi left review comments

@sappelhoff sappelhoff Awaiting requested review from sappelhoff

@effigies effigies Awaiting requested review from effigies

@franklin-feingold franklin-feingold Awaiting requested review from franklin-feingold

Assignees

@ChristophePhillips ChristophePhillips

Labels
enhancement PET
Projects
No open projects
BEP009: Positron Emission Tomography
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
10 participants
@melanieganz @hoechenberger @nellh @effigies @sappelhoff @tsalo @mnoergaard @rwblair @david-nishi @ChristophePhillips