Skip to content

check-setup: validate input datasets at their configured path_in_babs - #407

Open
yibeichan wants to merge 1 commit into
PennLINC:mainfrom
yibeichan:fix/check-setup-respects-path-in-babs
Open

check-setup: validate input datasets at their configured path_in_babs#407
yibeichan wants to merge 1 commit into
PennLINC:mainfrom
yibeichan:fix/check-setup-respects-path-in-babs

Conversation

@yibeichan

Copy link
Copy Markdown
Collaborator

Closes #325.

The bug

babs_check_setup probes a hardcoded <analysis>/inputs/data before validating
the input datasets:

temp_list = get_immediate_subdirectories(op.join(self.analysis_path, 'inputs/data'))

babs init clones each input dataset to its configured path_in_babs, so on any
project not using the default layout this raises a raw FileNotFoundError out of
os.listdir — not even the friendly ValueError on the line below, which only
fires for a directory that exists but is empty.

Since #369 this affects a layout BABS itself generates. The BIDS-study layout
puts inputs under sourcedata/<name>, so babs init writes:

input_datasets:
  BIDS:
    path_in_babs: 'sourcedata/BIDS'
  NIDM:
    path_in_babs: 'sourcedata/NIDM'

and the resulting analysis directory has sourcedata/{BIDS,NIDM} and no inputs/
at all. babs check-setup cannot run on such a project, which means losing the
test-job and environment checks that do work — we currently skip check-setup
entirely on every run.

The fix

The pre-check was already redundant. The per-dataset loop immediately below
resolves in_ds.babs_project_analysis_path and raises for a missing directory
or a non-DataLad dataset, using the configured path — so it covers everything
the hardcoded probe did, correctly.

The one condition the probe uniquely covered is "no input datasets configured",
which is a property of the config rather than of the filesystem. This PR checks
that directly and drops the path probe, so there is no second path-resolution
to keep in sync with path_in_babs.

get_immediate_subdirectories was imported here only for that call. It is now
unused within babs/, but its definition in utils.py is left in place — it is
public API and tests/test_utils.py::test_get_immediate_subdirectories covers it.

Verification

Against a real BIDS-study-layout project (path_in_babs: sourcedata/BIDS and
sourcedata/NIDM), on ea5d6f0:

OLD pre-check: FileNotFoundError -> .../analysis/inputs/data
NEW pre-check: passes, 2 input dataset(s)
   BIDS   path_in_babs=sourcedata/BIDS   exists=True is_datalad=True
   NIDM   path_in_babs=sourcedata/NIDM   exists=True is_datalad=True

ruff check and ruff format --check are clean on the changed file.

On tests: I did not add one. The existing tests/test_check_setup.py cases are
integration tests built on the babs_project_* fixtures and the simbids apptainer
image, which I can't run in my environment, and there is no study-layout fixture to
extend. Happy to add a regression test if you can point me at the fixture you'd want
it built on — a path_in_babs-varying project fixture would cover this directly.

Closes PennLINC#325.

`babs_check_setup` probed a hardcoded `<analysis>/inputs/data` before
validating each input dataset. `babs init` clones each input to its
configured `path_in_babs`, so any project not using the default layout
fails here with a raw FileNotFoundError from os.listdir:

    FileNotFoundError: [Errno 2] No such file or directory:
    '.../analysis/inputs/data'

Since PennLINC#369 this also affects a layout BABS itself generates: the
BIDS-study layout puts inputs under `sourcedata/<name>`, so
`babs check-setup` cannot run on those projects at all.

The pre-check was already redundant. The per-dataset loop immediately
below resolves `in_ds.babs_project_analysis_path` and raises for a
missing directory or a non-DataLad dataset, using the configured path.
The only condition the pre-check uniquely covered is "no input datasets
configured", which is a property of the config rather than of the
filesystem -- so check that directly.

`get_immediate_subdirectories` was imported here only for this call and
is now unused in babs/ (its definition in utils.py is left in place).

Verified against a real BIDS-study-layout project
(path_in_babs: sourcedata/BIDS, sourcedata/NIDM): the old pre-check
raises FileNotFoundError, the new one passes and both input datasets
then validate at their configured paths.
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 79.38%. Comparing base (ea5d6f0) to head (f685e8f).

Files with missing lines Patch % Lines
babs/check_setup.py 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #407      +/-   ##
==========================================
- Coverage   79.39%   79.38%   -0.01%     
==========================================
  Files          17       17              
  Lines        2169     2168       -1     
  Branches      384      384              
==========================================
- Hits         1722     1721       -1     
  Misses        302      302              
  Partials      145      145              

☔ View full report in Codecov by Harness.
📢 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

babs check-setup ignores path_in_babs config

3 participants