[ENH] Bep 005: Arterial Spin Labeling #669

sappelhoff · 2020-11-11T21:58:49Z

follow up of #652

closes #41

rendered draft: https://bids-specification.readthedocs.io/en/bep-005/

TCON agenda and notes: https://docs.google.com/document/d/1AVj8SvVnkNcInvYpljFQgc3mpkxnjVvKTbRVigMHKBs/edit

src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md

patsycle · 2020-11-24T15:17:24Z

@sappelhoff Hi Stefan, I guess the checks are unsuccessfull now due to the links added in the description of EchoTime and FlipAngle. IS that correct?

sappelhoff · 2020-11-24T15:32:09Z

Hi Stefan, I guess the checks are unsuccessfull now due to the links added in the description of EchoTime and FlipAngle. IS that correct?

that is correct, however one broken link can already be fixed: ./99-appendices/09-entities.md --> look at the ./ with that you say: look at the 99-appendices directory in the current directory, and find the 09-entities.md file.

However, you have to take into account from where you are linking to ./99-appendices/09-entities.md

--> you are doing that from the 04-modality-specific-files folder. ... and in that folder, there is no 99-appendices folder.

So you need to do ../99-appendices/09-entities.md, because ../ means: "look in the directory above the current one".

And there you'll find 99-appendices

src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md

sappelhoff · 2020-11-27T20:29:03Z

@patsycle you can temporarily edit this line:

bids-specification/readthedocs.yml

Line 15 in 727ee87

fail_on_warning: true

so that it says

 fail_on_warning: false

that way, the draft will continue to be built. Then once your missing "link" is available, we change that back to "true".

Also edit this line in the other file and remove the --strict

bids-specification/.circleci/config.yml

Line 14 in 727ee87

command: mkdocs build --clean --strict --verbose

Co-authored-by: Patricia Clement <[email protected]> Co-authored-by: Stefan Appelhoff <[email protected]> Co-authored-by: Chris Markiewicz <[email protected]> Co-authored-by: Julia Guiomar Niso Galán <[email protected]> Co-authored-by: Remi Gau <[email protected]>

based on comment Thijs Van Osch

Removed: all ancillary scaling factors should be taken into account in the conversion to BIDS, which is why BIDS does not provide a separate scaling factor field other than the NIfTI header.

added in the common - sequence specifics table Reason: for ASL, defining 2D and 3D is required. This used to be added in the PulseSequenceType field, but this should stay a free text field. Therefore, the MRAcquisitionType field is added (based on DicomTag).

The 2D/3D information needed for ASL, is now moved to the new MRAcquisitionType field.

based on comment of Stefan Appelhof, the need for a single column was added in the introductory text for aslcontext.tsv, before the 3 cases.

tsalo · 2021-01-05T18:31:50Z

Per today's maintainers call, I think this PR is stable enough that we can start working on the schema components. I think I will open a PR directly to bep-005 that the BEP group can review. Does that sound good?

effigies · 2021-01-05T18:40:38Z

I think I will open a PR directly to bep-005 that the BEP group can review.

That would have been my suggestion.

* Add ASL to schema files and regenerate entity table. * Fix style issue. * Add fmap-format m0scan to schema.

Added 'Referring to the M0 of blood'

src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md

Added additional information for LabelingDuration

effigies · 2021-01-13T19:11:46Z

@patsycle @HenkMutsaerts I just merged the final BEP-001 PR. That's causing some merge conflicts. I'm happy to resolve them unless you would prefer to do it.

HenkMutsaerts · 2021-01-14T08:47:18Z

@patsycle @HenkMutsaerts I just merged the final BEP-001 PR. That's causing some merge conflicts. I'm happy to resolve them unless you would prefer to do it.

The conflicts look easy resolvable, I don't mind who does it :) I'm working on making ExploreASL read ASL-BIDS, in combination with bids-matlab, then we can really process all our BIDS ASL examples, which I like as a final test before merging bep005, should be done hopefully around this weekend.

sappelhoff · 2021-01-14T08:51:53Z

@HenkMutsaerts @patsycle we can also merge this PR (after @effigies resolved conflicts, thanks for offering), and wait with the release for a few days.

that would give you the chance to test everything for a few more days
if during these days (before the release) you notice some things that need to be remedied, you can make a small edit PR

The big benefit would be that we get this huge PR merged in, and that other PRs can continue more easily without being concerned about merge conflicts being introduced here and there.

I don't see a drawback to that approach, does anyone object?

patsycle · 2021-01-14T08:55:55Z

@sappelhoff Before doing that, we were thinking about changing one final thing (e.g. *_labeling.jpg to *_asllabeling.jpg, for consistency with the naming of aslcontext.tsv). I will do that today. But, how do I change this for the schemas, or could @tsalo help me out with this?

HenkMutsaerts · 2021-01-14T08:59:58Z

@HenkMutsaerts @patsycle we can also merge this PR (after @effigies resolved conflicts, thanks for offering), and wait with the release for a few days.

that would give you the chance to test everything for a few more days

if during these days (before the release) you notice some things that need to be remedied, you can make a small edit PR

The big benefit would be that we get this huge PR merged in, and that other PRs can continue more easily without being concerned about merge conflicts being introduced here and there.

I don't see a drawback to that approach, does anyone object?

No, this seems a good idea!

effigies · 2021-01-14T18:32:18Z

Conflicts resolved and appendix renumbered, since qMRI got XI. Please review the changes to be sure I didn't accidentally revert something I shouldn't have.

patsycle · 2021-01-15T08:41:39Z

@effigies It all looks good, couldn't find any errors. @sappelhoff Linkchecker is failing though..

sappelhoff · 2021-01-15T09:21:09Z

@patsycle I re-ran the linkchecker and it passed the second time. That issue was only temporary.

Now the BEP is merged 🎉 thanks to the team and the reviewers! Now - as suggested - you can make some final checks together with @HenkMutsaerts and his pipeline ... and if everything is fine, we can release soon, because BEP001 has also been merged.

That'll be a big celebration with BEP001 and BEP005 in for BIDS 1.5 :-)

sappelhoff · 2021-01-15T09:25:14Z

PS: Very sorry to merge this so early in the morning ... The champagne from the fridge would taste much better in the evening 😄

patsycle · 2021-01-15T10:23:49Z

PS: Very sorry to merge this so early in the morning ... The champagne from the fridge would taste much better in the evening 😄

@sappelhoff No worries, it is already noon here. Perfect timing to open a bottle ;)

Remi-Gau · 2023-04-28T20:50:42Z

src/schema/datatypes/perf.yaml

+- suffixes:
+    - aslcontext
+  extensions:
+    - .tsv
+    - .json


only tsv should be allowed

sappelhoff requested a review from chrisgorgo as a code owner November 11, 2020 21:58

sappelhoff requested review from HenkMutsaerts and patsycle November 11, 2020 21:59

sappelhoff mentioned this pull request Nov 11, 2020

[ENH] Bep 005: Arterial Spin Labeling #652

Merged

This was referenced Nov 16, 2020

[ENH] BEP001 - RepetitionTimeExcitation and RepetitionTimePreparation #671

Merged

[ENH] BEP001 - New entity: flip #672

Merged

gllmflndn reviewed Nov 16, 2020

View reviewed changes

src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Outdated Show resolved Hide resolved

sappelhoff added the BEP label Nov 18, 2020

patsycle reviewed Nov 19, 2020

View reviewed changes

src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Outdated Show resolved Hide resolved

patsycle reviewed Nov 19, 2020

View reviewed changes

src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Outdated Show resolved Hide resolved

tsalo reviewed Nov 20, 2020

View reviewed changes

src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Outdated Show resolved Hide resolved

tsalo reviewed Nov 20, 2020

View reviewed changes

src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Outdated Show resolved Hide resolved

effigies added this to the 1.5.0 milestone Nov 20, 2020

sappelhoff force-pushed the bep-005 branch from d3808e1 to d344e87 Compare November 23, 2020 09:39

patsycle reviewed Nov 27, 2020

View reviewed changes

src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Outdated Show resolved Hide resolved

HenkMutsaerts and others added 12 commits November 30, 2020 10:59

Update table errors

a5896b9

correction for table issues 'common metadata for PCASL/PASL'

d359424

correction latin phrases

946bc95

Corrections tables

4dd6d30

correction tables Cases

79b44f5

added link pepolar

d30d048

deleted 'don't discriminate between types of labeling

ee41299

based on comment Thijs Van Osch

Moved MagneticFieldStrenght asl requirement to common

6cf03bd

removed scaling factor info based on comment gllmflndn

1ae383c

Removed: all ancillary scaling factors should be taken into account in the conversion to BIDS, which is why BIDS does not provide a separate scaling factor field other than the NIfTI header.

added MRAcquisitionType

a15460d

added in the common - sequence specifics table Reason: for ASL, defining 2D and 3D is required. This used to be added in the PulseSequenceType field, but this should stay a free text field. Therefore, the MRAcquisitionType field is added (based on DicomTag).

removed PulseSequenceType from the ASL part

3e12993

The 2D/3D information needed for ASL, is now moved to the new MRAcquisitionType field.

added single column requirement for aslcontext.tsv

061455b

based on comment of Stefan Appelhof, the need for a single column was added in the introductory text for aslcontext.tsv, before the 3 cases.

tsalo mentioned this pull request Jan 5, 2021

[SCHEMA] Add ASL to schema #703

Merged

patsycle and others added 3 commits January 9, 2021 14:17

Changed VascularCrushingVENC to VascularCrushingVenc

b26b385

[SCHEMA] Add ASL to schema (#703)

d623aee

* Add ASL to schema files and regenerate entity table. * Fix style issue. * Add fmap-format m0scan to schema.

update M0Estimate fields

df40765

Added 'Referring to the M0 of blood'

effigies reviewed Jan 12, 2021

View reviewed changes

src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md Outdated Show resolved Hide resolved

update LabelingDuration

7222b1f

Added additional information for LabelingDuration

effigies mentioned this pull request Jan 13, 2021

Suggestions for BEP-009 #708

Merged

update labeling.jpg into *_asllabeling.jpg

185ded5

tsalo mentioned this pull request Jan 14, 2021

Change labeling.jpg to asllabeling.jpg #709

Merged

tsalo and others added 3 commits January 14, 2021 15:29

Change labeling to asllabeling. (#709)

3ad4fd1

Merge remote-tracking branch 'upstream/master' into bep-005

1e42f6d

RF: Renumber ASL appendix to XII

b92b6ea

sappelhoff merged commit d794736 into master Jan 15, 2021

loj mentioned this pull request Jan 19, 2021

NF: add asl/perf to known modalities/datatypes psychoinformatics-de/hirni-toolbox#16

Merged

agahkarakuzu mentioned this pull request Jun 15, 2021

[ENH] BEP001 - New entities: inv & mt #681

Merged

Remi-Gau reviewed Apr 28, 2023

View reviewed changes

MeenuyD mentioned this pull request Apr 24, 2024

Fix the broken link reported by CI bids-standard/bids-website#388

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ENH] Bep 005: Arterial Spin Labeling #669

[ENH] Bep 005: Arterial Spin Labeling #669

sappelhoff commented Nov 11, 2020 •

edited

Loading

patsycle commented Nov 24, 2020

sappelhoff commented Nov 24, 2020

sappelhoff commented Nov 27, 2020

tsalo commented Jan 5, 2021

effigies commented Jan 5, 2021

effigies commented Jan 13, 2021

HenkMutsaerts commented Jan 14, 2021

sappelhoff commented Jan 14, 2021

patsycle commented Jan 14, 2021

HenkMutsaerts commented Jan 14, 2021

effigies commented Jan 14, 2021

patsycle commented Jan 15, 2021

sappelhoff commented Jan 15, 2021

sappelhoff commented Jan 15, 2021

patsycle commented Jan 15, 2021

Remi-Gau Apr 28, 2023

[ENH] Bep 005: Arterial Spin Labeling #669

[ENH] Bep 005: Arterial Spin Labeling #669

Conversation

sappelhoff commented Nov 11, 2020 • edited Loading

rendered draft: https://bids-specification.readthedocs.io/en/bep-005/

patsycle commented Nov 24, 2020

sappelhoff commented Nov 24, 2020

sappelhoff commented Nov 27, 2020

tsalo commented Jan 5, 2021

effigies commented Jan 5, 2021

effigies commented Jan 13, 2021

HenkMutsaerts commented Jan 14, 2021

sappelhoff commented Jan 14, 2021

patsycle commented Jan 14, 2021

HenkMutsaerts commented Jan 14, 2021

effigies commented Jan 14, 2021

patsycle commented Jan 15, 2021

sappelhoff commented Jan 15, 2021

sappelhoff commented Jan 15, 2021

patsycle commented Jan 15, 2021

Remi-Gau Apr 28, 2023

Choose a reason for hiding this comment

sappelhoff commented Nov 11, 2020 •

edited

Loading