Adding an option to fit BIDS-study layout (updated) by djarecka · Pull Request #369 · PennLINC/babs

djarecka · 2026-04-17T17:04:02Z

instead of #361 (after rewriting history went wrong); I included the newest changes to main.
From the previous description:

allowing for configuring analysis_path
ading rias to .babs and gitignore
adding initial configuration file to .babs, so it can be read whenever is neeeded

An example presenting the usage and the layout can be found on the
babs_demo repo, follow the section for the bids_layout

…rectory for config/RIA storage

codecov-commenter · 2026-04-17T17:13:54Z

Codecov Report

❌ Patch coverage is 87.87879% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.12%. Comparing base (95fd99d) to head (42a8cea).

Files with missing lines	Patch %	Lines
babs/bootstrap.py	71.42%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #369      +/-   ##
==========================================
+ Coverage   79.07%   79.12%   +0.04%     
==========================================
  Files          17       17              
  Lines        2079     2103      +24     
  Branches      362      366       +4     
==========================================
+ Hits         1644     1664      +20     
- Misses        300      302       +2     
- Partials      135      137       +2

☔ 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.

tien-tong · 2026-04-23T13:05:37Z

+        processing_level,
+        system,
+        project_root=None,
+        analysis_dir='analysis',


new arg but docstring wasn't updated

asmacdo

I ran this against the https://github.com/djarecka/babs_demo with the bids layout setup, and it worked very smoothly!

Tested this end-to-end on a Dartmouth/Discovery slurm cluster using djarecka's babs_demo bids_layout walkthrough (simbids → init with analysis_path: . and .babs/{input,output}_ria → submit → merge → unzip). All jobs finished and the resulting layout under <study>/derivatives/<project>/ is exactly the BIDS-derivatives shape we (mechababs) want for the OpenNeuroStudies/OpenNeuroDerivatives integration. Strongly aligned with our roadmap — thanks!

Three small things noticed while verifying:

1. .gitattributes defaults annex small metadata files. .babs/babs_init_config.yaml (newly committed by this PR) and the extracted dataset_description.json both end up as annex symlinks. Filed as #378
2. .gitignore RIA rule uses op.basename, which is over-broad. With input_ria_path: .babs/input_ria, the basename input_ria becomes a gitignore rule that matches any path component named input_ria. I think this is just a nitpick and should be fine.

djarecka · 2026-05-28T15:38:55Z

Hi @tien-tong, just pinging if you have any questions or comments to this PR

tien-tong

I've tested the orig BABS project layout with real data and everything looks good!

I've also tested a BIDS-study layout config yaml file for freesurfer-post (my commit). This section from the BABSProject/README.md need to be changed to sourcedata for BIDS layout

## Dataset structure

- All inputs (i.e. building blocks from other sources) are located in
  `inputs/`.

Other than that, everything looks good to me!

tien-tong · 2026-05-21T16:12:23Z

+
+        analysis_dir = cfg.get('analysis_path', 'analysis')
+        self.analysis_path = op.normpath(op.join(self.project_root, analysis_dir))
        self._analysis_datalad_handle = None


analysis_path, input_ria_path, and output_ria_path are joined with project_root without validation. Path values are not validated, so absolute paths, .., "", and "." can escape or collapse into unintended locations

if a config uses analysis_path: ../outside, BABS may create/write outside the project root, while cleanup only deletes project_root.

Suggested fix: centralize path resolution with Path.resolve(), reject empty/non-string paths, reject absolute paths unless explicitly supported, and require resolved paths to stay inside project_root.

tien-tong · 2026-05-21T16:18:02Z

            system,
            project_root=op.dirname(self.analysis_path),
+            analysis_dir=op.basename(self.analysis_path),
            shared_group_mode=shared_group_mode,


nested analysis_path are reconstructed from dirname + basename

but PROJECT_ROOT in the generated environment still means the real project root.

CONTAINER_SHARED="${PROJECT_ROOT}/{{ analysis_dir }}/${CONTAINER_JOB}"

This is fragile and confusing, and breaks easily for external callers or future script changes.

Suggested fix: pass a single analysis_path or analysis_relpath into the template. Rename shell variables to what they mean, e.g. BABS_PROJECT_ROOT and BABS_ANALYSIS_PATH, and stop reconstructing nested paths from basename.

tien-tong · 2026-05-21T16:34:53Z

+        # not to track input/output RIA stores:
+        gitignore_file.write('\n' + op.basename(self.input_ria_path))
+        gitignore_file.write('\n' + op.basename(self.output_ria_path))
        # not to track `logs` folder:


.gitignore only ignores RIA basenames.

Suggested fix: when RIA paths are inside analysis_path, write paths relative to analysis_path.

tien-tong · 2026-05-21T16:37:30Z


-    babs_proj = BABSBootstrap(project_root)
+    babs_proj = BABSBootstrap(project_root, container_config=container_config)
    try:


Direct API usage like silently ignores custom analysis_path.

class BABSBootstrap(BABS): def __init__(self, project_root, container_config=None): super().__init__(project_root, container_config=container_config)

Suggested fix: either require container_config in BABSBootstrap.__init__ or resolve init paths inside babs_bootstrap() before any path-dependent work.

tien-tong · 2026-05-21T16:39:04Z

+            if op.exists(root_config_path):
+                with open(root_config_path) as f:
+                    cfg = yaml.safe_load(f) or {}
+


empty YAML returns None, causing an opaque AttributeError.

Suggested fix: validate with something like:

cfg = yaml.safe_load(f) or {} if not isinstance(cfg, dict): raise ValueError('container_config must be a YAML mapping')

tien-tong · 2026-05-21T16:41:29Z


-cmd_template: '{{ submit_head }} {{ env_flags }} {{ name_flag_str }}{{ job_name }} {{ eo_args }} {{ array_args }} {% if test %}{{ babs.analysis_path }}/code/check_setup/call_test_job.sh{% else %}{{ babs.analysis_path }}/code/participant_job.sh {{ dssource }} {{ pushgitremote }} {{ babs.job_submit_path_abs }} {{ babs.project_root }}{% endif %}'
+cmd_template: '{{ submit_head }} {{ env_flags }} {{ name_flag_str }}{{ job_name }} {{ eo_args }} {{ array_args }} {% if test %}{{ babs.analysis_path }}/code/check_setup/call_test_job.sh{% else %}{{ babs.analysis_path }}/code/participant_job.sh {{ dssource }} {{ pushgitremote }} {{ babs.job_submit_path_abs }} {{ babs.analysis_root }}{% endif %}'
 job_name_template: '{{ job_name }}'


generated command strings are split with plain .split().

job_id = sbatch_get_job_id(cmd.split(), analysis_path)

spaces in project paths, RIA paths, or analysis paths will break job submission.

Suggested fix: build argv as structured data, or render shell-quoted strings and parse with shlex.split().

tien-tong · 2026-05-21T16:43:54Z

        with open(self.config_path, 'w') as f:
            f.write(
                template.render(
+                    analysis_dir=op.basename(self.analysis_path),


analysis_dir is passed but unused

Add custom analysis_path support: configurable analysis dir, .babs di…

9bb4256

…rectory for config/RIA storage

ruff fix

3535cd8

tien-tong self-requested a review April 23, 2026 13:04

tien-tong reviewed Apr 23, 2026

View reviewed changes

tien-tong added 3 commits April 23, 2026 09:27

Merge branch 'main' into structurechange_1

24570d1

Apply Ruff formatting to babs/base.py

6295329

fix test

42a8cea

tien-tong linked an issue Apr 24, 2026 that may be closed by this pull request

Are RIAs necessary? #357

Closed

asmacdo mentioned this pull request May 21, 2026

Default .gitattributes should be BIDS-friendly #378

Open

tien-tong self-requested a review May 21, 2026 15:59

asmacdo approved these changes May 21, 2026

View reviewed changes

asmacdo mentioned this pull request May 21, 2026

Make logs/ entry in .gitignore optional #372

Open

tien-tong added the enhancement New feature or request label May 22, 2026

This was referenced Jun 4, 2026

Target setups thought dump asmacdo/mechababs#4

Open

contain babs config within result code/ asmacdo/mechababs#7

Open

Epic: upstream babs dependencies (M2) asmacdo/mechababs#38

Open

Add freesurfer-post BIDS study layout example

39b58f7

tien-tong requested changes Jun 8, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding an option to fit BIDS-study layout (updated)#369

Adding an option to fit BIDS-study layout (updated)#369
djarecka wants to merge 6 commits into
PennLINC:mainfrom
djarecka:structurechange_1

djarecka commented Apr 17, 2026

Uh oh!

codecov-commenter commented Apr 17, 2026 •

edited

Loading

Uh oh!

tien-tong Apr 23, 2026

Uh oh!

asmacdo left a comment

Uh oh!

djarecka commented May 28, 2026

Uh oh!

tien-tong left a comment

Uh oh!

tien-tong May 21, 2026

Uh oh!

tien-tong May 21, 2026

Uh oh!

tien-tong May 21, 2026

Uh oh!

tien-tong May 21, 2026

Uh oh!

tien-tong May 21, 2026

Uh oh!

tien-tong May 21, 2026

Uh oh!

tien-tong May 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

djarecka commented Apr 17, 2026

Uh oh!

codecov-commenter commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asmacdo left a comment

Choose a reason for hiding this comment

Uh oh!

djarecka commented May 28, 2026

Uh oh!

tien-tong left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-commenter commented Apr 17, 2026 •

edited

Loading