Skip to content

Conversation

@yarikoptic
Copy link
Collaborator

@yarikoptic yarikoptic commented Aug 21, 2025

  • we really should not advocate having _ as a separator, so I made example to use 'pipeline1-v1' and 'pipeline2`. This should make it much more consistent with how we name in BIDS where '_' is a separator between entities.
  • we had and <pipeline_name> (as we used in other places). I unified to <pipeline_name> although would not mind going to or introducing (expclitly) that '' means '<pipeline_name>[-]'

initially prompted by looking at oddly looking example in common principles with the pipeline_1

- we really should not advocate having '_' as a separator, so I made example to use
  'pipeline1-v1' and 'pipeline2.  This should make it much more consistent
  with how we name in BIDS where '_' is a separator between entities.
- we had <pipeline> and <pipeline_name> (as we used in other places).
  I unified to <pipeline_name> although would not mind going to <pipeline>
  or introducing (expclitly) that '<pipeline>' means '<pipeline_name>[-<variant>]'
@codecov
Copy link

codecov bot commented Aug 21, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.71%. Comparing base (db1c087) to head (944f634).
⚠️ Report is 149 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2177   +/-   ##
=======================================
  Coverage   82.71%   82.71%           
=======================================
  Files          20       20           
  Lines        1608     1608           
=======================================
  Hits         1330     1330           
  Misses        278      278           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member

@julia-pfarr julia-pfarr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about just changing <pipeline_name> to <pipeline-name> everywhere? This should make it clear that no _ should be used anywhere in the name. Or what you suggested

introducing (expclitly) that '' means '<pipeline_name>[-]'

works, too. Either way, I would make it more explicit.

@yarikoptic
Copy link
Collaborator Author

it would indeed be nice to harmonize but we ATM use both

_ : run_index,source_entities,participant_label
❯ git grep '<[a-zA-Z0-9]*_[a-zA-Z0-9]*>' src/**/*.md | grep -v pipeline_name | nl
     1	src/CHANGES.md:-   Added `run-<run_index>` to the phase encoding maps. Improved the description.
     2	src/CHANGES.md:-   Added missing `sub-<participant_label>` in behavioral data filenames.
     3	src/appendices/hed.md:A library schema is specified in the form `<library-name<_>library-version>`.
     4	src/derivatives/common-data-types.md:                <source_entities>[_space-<space>][_desc-<label>]_<suffix>.<extension>
     5	src/derivatives/imaging.md:                <source_entities>[_space-<space>][_res-<label>][_den-<label>][_desc-<label>]_<suffix>.<extension>
     6	src/derivatives/imaging.md:                <source_entities>[_space-<space>][_res-<label>][_label-<label>][_desc-<label>]_mask.json
     7	src/derivatives/imaging.md:                <source_entities>[_space-<space>][_res-<label>][_label-<label>][_desc-<label>]_mask.nii[.gz]
     8	src/derivatives/imaging.md:                <source_entities>[_space-<space>][_seg-<label>][_res-<label>][_desc-<label>]_dseg.json
     9	src/derivatives/imaging.md:                <source_entities>[_space-<space>][_seg-<label>][_res-<label>][_desc-<label>]_dseg.nii[.gz]
    10	src/derivatives/imaging.md:                <source_entities>[_space-<space>][_seg-<label>][_res-<label>][_desc-<label>]_dseg.tsv
    11	src/derivatives/imaging.md:                <source_entities>[_space-<space>][_seg-<label>][_res-<label>][_label-<label>][_desc-<label>]_probseg.json
    12	src/derivatives/imaging.md:                <source_entities>[_space-<space>][_seg-<label>][_res-<label>][_label-<label>][_desc-<label>]_probseg.nii[.gz]
    13	src/derivatives/imaging.md:                <source_entities>[_hemi-{L|R}][_space-<space>][_seg-<label>][_res-<label>][_den-<label>][_desc-<label>]_dseg.json
    14	src/derivatives/imaging.md:                <source_entities>[_hemi-{L|R}][_space-<space>][_seg-<label>][_den-<label>][_desc-<label>]_dseg.label.gii
    15	src/derivatives/imaging.md:                <source_entities>[_hemi-{L|R}][_space-<space>][_seg-<label>][_res-<label>][_den-<label>][_desc-<label>]_dseg.dlabel.nii
    16	src/derivatives/imaging.md:                <source_entities>[_hemi-{L|R}][_space-<space>][_seg-<label>][_res-<label>][_den-<label>][_desc-<label>]_dseg.tsv
    17	src/derivatives/introduction.md:    `<source_entities>[_keyword-<value>]_<suffix>.<extension>`
    18	src/pregh-changes.md:-   Added `run-<run_index>` to the phase encoding maps. Improved the description.
    19	src/pregh-changes.md:-   Added missing `sub-<participant_label>` in behavioral data filenames.
-: relative-path, dataset-name
❯ git grep '<[a-zA-Z0-9]*-[a-zA-Z0-9]*>' src/**/*.md | grep -v pipeline_name | nl
     1	src/common-principles.md:bids:[<dataset-name>]:<relative-path>
     2	src/common-principles.md:which defines a `path` component of the form `<dataset-name>:<relative-path>`.
     3	src/common-principles.md:`[<dataset-name>]:<relative-path>`.
     4	src/schema/README.md:| `"bids-uri"` | A URI of the form `bids:<dataset>:<relative-path>`. If `<dataset>` is empty, the current dataset is used. | `exists('bids::participants.tsv', 'bids-uri')`                                        |

so which way should we go? ;-)

@julia-pfarr
Copy link
Member

Following Research Data Management conventions, dash should be used to represent an entity ("dataset-name") and an underscore should be used to separate entities ("dataset-name_participant-label". So I vote for using dashes

@effigies
Copy link
Collaborator

run_index,source_entities,participant_label

Note that run_index and participant_label appear in the changelog, not the spec. source_entities is the only one in the spec.

I would be fine with switching to source-entities if this matters.

…ormity

=== Do not change lines below ===
{
 "chain": [],
 "cmd": "git-sedi pipeline_name pipeline-name",
 "exit": 0,
 "extra_inputs": [],
 "inputs": [],
 "outputs": [],
 "pwd": "."
}
^^^ Do not change lines above ^^^
…uniformity

=== Do not change lines below ===
{
 "chain": [],
 "cmd": "git-sedi source_entities source-entities",
 "exit": 0,
 "extra_inputs": [],
 "inputs": [],
 "outputs": [],
 "pwd": "."
}
^^^ Do not change lines above ^^^
@effigies effigies added this to the 1.10.1 milestone Aug 27, 2025
@effigies effigies added the exclude-from-changelog This item will not feature in the automatically generated changelog label Aug 28, 2025
@effigies
Copy link
Collaborator

effigies commented Sep 3, 2025

Ugh. I forgot to merge this for the release. Sorry.

@effigies effigies merged commit 6070069 into bids-standard:master Sep 3, 2025
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation exclude-from-changelog This item will not feature in the automatically generated changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants