[FIX] Move `_stim.<ext>` specification within the Task Events module #1750

oesteban · 2024-03-25T10:15:22Z

This fix builds on #1749 to separate _stim and _physio in the specs.

Currently _stim files are specified sideways as an addon or extra branch of _physio, which can be confusing and misrepresent their intent.

Moving them with the other stimuli definitions increases the consistency of the spec and the findability of _stim specifications.

This fix requires #1749 for a more consistent prescription of tsv.gz files.

src/common-principles.md

src/modality-specific-files/task-events.md

sappelhoff · 2024-03-26T17:13:19Z

This fix builds on #1748 to separate _stim and _physio in the specs.

did you mean to reference:

[ENH] Add compressed TSV files to the common principles #1749 instead?

sappelhoff

Currently _stim files are specified sideways as an addon or extra branch of _physio, which can be confusing and misrepresent their intent.

Moving them with the other stimuli definitions increases the consistency of the spec and the findability of _stim specifications.

agreed!

Let's fix up #1749 first and then get this one ready!

src/modality-specific-files/task-events.md

src/modality-specific-files/physiological-recordings.md

oesteban · 2024-03-27T13:06:29Z

Since #1749 is looking very close to the finish line, the following link oesteban/bids-specification@fix/large-tabular-files...oesteban:bids-specification:fix/stim-files-with-stimuli can be used to look at the changes specific of this PR without distractions.

effigies

LGTM. Small comments.

src/modality-specific-files/physiological-recordings.md

tsalo

I just have one small concern/question about inheritance with .tsv.gz files.

src/common-principles.md

src/modality-specific-files/task-events.md

codecov · 2024-04-02T09:17:53Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.93%. Comparing base (dbcb237) to head (cd9c926).

❗ Current head cd9c926 differs from pull request most recent head 00114cc. Consider uploading reports for the commit 00114cc to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1750   +/-   ##
=======================================
  Coverage   87.93%   87.93%           
=======================================
  Files          16       16           
  Lines        1351     1351           
=======================================
  Hits         1188     1188           
  Misses        163      163

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Remi-Gau · 2024-04-02T09:48:10Z

I introduced a merged conflict. Will fix.

oesteban · 2024-04-02T09:49:16Z

I introduced a merged conflict. Will fix.

I can take care, will be faster

EDIT - fixed!

src/modality-specific-files/physiological-recordings.md

Co-authored-by: Remi Gau <remi_gau@hotmail.com>

Remi-Gau

Good with me

oesteban · 2024-04-02T15:07:59Z

Good with me

What should we do about the admonition linting? I haven't managed to ignore them or making the plugin work.

oesteban · 2024-04-02T15:09:02Z

Perhaps moving them inside the admonition?

oesteban · 2024-04-02T15:16:41Z

Perhaps moving them inside the admonition?

That actually did the trick!

src/modality-specific-files/task-events.md

sappelhoff

I agree with the changes here in principle, pending this review item:

https://github.com/bids-standard/bids-specification/pull/1750/files#r1548105139

it'd be important that the reference to an example points at a valid example

src/modality-specific-files/task-events.md

Remi-Gau · 2024-04-03T08:29:41Z

I think this should be ready to merge after the 5 days period.

Remi-Gau · 2024-04-03T08:40:17Z

I agree with the changes here in principle, pending this review item:
* https://github.com/bids-standard/bids-specification/pull/1750/files#r1548105139
it'd be important that the reference to an example points at a valid example

BTW: did a quick check on openneuro via the datalad superdataset and I could not find a dataset with stim files except the one that @oesteban had mentioned.

oesteban · 2024-04-11T16:32:52Z

@Remi-Gau @sappelhoff @effigies - may I merge this? I'd like to rebase BEP020, which will benefit substantially from getting stim out of physio.

@effigies

* origin/master: (278 commits) [FIX] Move ``_stim.<ext>`` specification within the Task Events module (bids-standard#1750) rm macros with missing metadata Update src/schema/objects/metadata.yaml Update src/schema/objects/metadata.yaml Update src/schema/objects/metadata.yaml [pre-commit.ci] pre-commit autoupdate Update src/common-principles.md Update src/common-principles.md Update src/common-principles.md Update src/common-principles.md fix: replace SHOULD with MUST in last changes (cc @effigies) Apply suggestions from code review Apply suggestions from code review Apply suggestions from code review fix: checking if chinese characters are accepted by pandoc Apply suggestions from code review Update src/common-principles.md Update src/schema/rules/checks/events.yaml fix: deduplicate prescriptions for columns in tsvgz Apply suggestions from code review ... Conflicts: mkdocs.yml - just redid changes found in prior diff but unifying for current indentation and use of ""

oesteban requested review from effigies, Remi-Gau and sappelhoff March 25, 2024 10:15

oesteban force-pushed the fix/stim-files-with-stimuli branch from c004c7a to efe5c2a Compare March 25, 2024 10:41

oesteban mentioned this pull request Mar 25, 2024

[ENH] BEP 020 Eye Tracking #1128

Open

9 tasks

oesteban force-pushed the fix/stim-files-with-stimuli branch 3 times, most recently from 27d13ad to 667e373 Compare March 25, 2024 15:13

Remi-Gau reviewed Mar 25, 2024

View reviewed changes

src/common-principles.md Outdated Show resolved Hide resolved

Remi-Gau reviewed Mar 26, 2024

View reviewed changes

src/modality-specific-files/task-events.md Outdated Show resolved Hide resolved

sappelhoff reviewed Mar 26, 2024

View reviewed changes

oesteban commented Mar 27, 2024

View reviewed changes

src/modality-specific-files/task-events.md Outdated Show resolved Hide resolved

oesteban commented Mar 27, 2024

View reviewed changes

src/modality-specific-files/physiological-recordings.md Outdated Show resolved Hide resolved

oesteban force-pushed the fix/stim-files-with-stimuli branch from 5054e2d to 78ea018 Compare March 27, 2024 13:00

effigies approved these changes Mar 27, 2024

View reviewed changes

src/modality-specific-files/physiological-recordings.md Outdated Show resolved Hide resolved

src/modality-specific-files/physiological-recordings.md Show resolved Hide resolved

oesteban mentioned this pull request Mar 27, 2024

[Potential BUG] Task events is modality-specific? #1751

Open

oesteban marked this pull request as ready for review March 27, 2024 15:29

oesteban requested a review from DimitriPapadopoulos as a code owner March 27, 2024 15:29

oesteban mentioned this pull request Mar 28, 2024

[BUG] Where are physio files placed when simultaneously recording (e.g. fMRI + EEG) #1753

Open

tsalo reviewed Mar 28, 2024

View reviewed changes

src/common-principles.md Outdated Show resolved Hide resolved

src/common-principles.md Outdated Show resolved Hide resolved

oesteban mentioned this pull request Mar 28, 2024

[ENH] Add compressed TSV files to the common principles #1749

Merged

oesteban force-pushed the fix/stim-files-with-stimuli branch from fdd99e0 to 328976a Compare April 2, 2024 08:57

oesteban commented Apr 2, 2024

View reviewed changes

src/modality-specific-files/task-events.md Show resolved Hide resolved

oesteban commented Apr 2, 2024

View reviewed changes

src/modality-specific-files/task-events.md Outdated Show resolved Hide resolved

oesteban requested a review from Remi-Gau April 2, 2024 09:21

Remi-Gau reviewed Apr 2, 2024

View reviewed changes

src/modality-specific-files/physiological-recordings.md Outdated Show resolved Hide resolved

Remi-Gau reviewed Apr 2, 2024

View reviewed changes

src/modality-specific-files/physiological-recordings.md Outdated Show resolved Hide resolved

Remi-Gau reviewed Apr 2, 2024

View reviewed changes

src/modality-specific-files/physiological-recordings.md Outdated Show resolved Hide resolved

oesteban and others added 3 commits April 2, 2024 14:19

Apply suggestions from code review

8273f66

Co-authored-by: Remi Gau <remi_gau@hotmail.com>

fix: typo

a9b70f8

fix: consistency and [recording-<label>] soft removal

5a4f292

oesteban force-pushed the fix/stim-files-with-stimuli branch from fb8dd81 to 5a4f292 Compare April 2, 2024 12:42

enh: add synthetic example

b30a696

oesteban mentioned this pull request Apr 2, 2024

FIX: Markdown linter throws false positives if definitions are only used inside admonition blocks #1760

Closed

oesteban force-pushed the fix/stim-files-with-stimuli branch from 4e8a6df to cd9c926 Compare April 2, 2024 13:36

Remi-Gau approved these changes Apr 2, 2024

View reviewed changes

fix: defining links inside admonition

cad7b48

oesteban force-pushed the fix/stim-files-with-stimuli branch from 07a34f0 to cad7b48 Compare April 2, 2024 15:12

Remi-Gau reviewed Apr 2, 2024

View reviewed changes

src/modality-specific-files/task-events.md Outdated Show resolved Hide resolved

Remi-Gau reviewed Apr 2, 2024

View reviewed changes

src/modality-specific-files/task-events.md Outdated Show resolved Hide resolved

sappelhoff reviewed Apr 2, 2024

View reviewed changes

oesteban commented Apr 2, 2024

View reviewed changes

Apply suggestions from code review

918a84a

oesteban commented Apr 2, 2024

View reviewed changes

src/modality-specific-files/task-events.md Show resolved Hide resolved

Update src/modality-specific-files/task-events.md

00114cc

sappelhoff approved these changes Apr 3, 2024

View reviewed changes

effigies merged commit 943c20e into bids-standard:master Apr 11, 2024
23 of 24 checks passed

sappelhoff changed the title ~~FIX: Move _stim.<ext> specification within the Task Events module~~ [FIX] Move _stim.<ext> specification within the Task Events module Jun 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FIX] Move `_stim.<ext>` specification within the Task Events module #1750

[FIX] Move `_stim.<ext>` specification within the Task Events module #1750

oesteban commented Mar 25, 2024 •

edited by sappelhoff

Loading

sappelhoff commented Mar 26, 2024

sappelhoff left a comment

oesteban commented Mar 27, 2024

effigies left a comment

tsalo left a comment

codecov bot commented Apr 2, 2024 •

edited

Loading

Remi-Gau commented Apr 2, 2024

oesteban commented Apr 2, 2024 •

edited

Loading

Remi-Gau left a comment

oesteban commented Apr 2, 2024

oesteban commented Apr 2, 2024

oesteban commented Apr 2, 2024

sappelhoff left a comment

Remi-Gau commented Apr 3, 2024

Remi-Gau commented Apr 3, 2024

oesteban commented Apr 11, 2024

[FIX] Move _stim.<ext> specification within the Task Events module #1750

[FIX] Move _stim.<ext> specification within the Task Events module #1750

Conversation

oesteban commented Mar 25, 2024 • edited by sappelhoff Loading

sappelhoff commented Mar 26, 2024

sappelhoff left a comment

Choose a reason for hiding this comment

oesteban commented Mar 27, 2024

effigies left a comment

Choose a reason for hiding this comment

tsalo left a comment

Choose a reason for hiding this comment

codecov bot commented Apr 2, 2024 • edited Loading

Codecov Report

Remi-Gau commented Apr 2, 2024

oesteban commented Apr 2, 2024 • edited Loading

Remi-Gau left a comment

Choose a reason for hiding this comment

oesteban commented Apr 2, 2024

oesteban commented Apr 2, 2024

oesteban commented Apr 2, 2024

sappelhoff left a comment

Choose a reason for hiding this comment

Remi-Gau commented Apr 3, 2024

Remi-Gau commented Apr 3, 2024

oesteban commented Apr 11, 2024

[FIX] Move `_stim.<ext>` specification within the Task Events module #1750

[FIX] Move `_stim.<ext>` specification within the Task Events module #1750

oesteban commented Mar 25, 2024 •

edited by sappelhoff

Loading

codecov bot commented Apr 2, 2024 •

edited

Loading

oesteban commented Apr 2, 2024 •

edited

Loading