Skip to content

Conversation

@tsalo
Copy link
Member

@tsalo tsalo commented May 28, 2025

Closes #549.

Changes proposed in this pull request

  • Remove out_basename input from BASILCBF interface and just use the cwd as the output directory.

@tsalo tsalo added the bug Something isn't working label May 28, 2025
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.90%. Comparing base (5bbe278) to head (cd8816a).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #552      +/-   ##
==========================================
- Coverage   75.91%   75.90%   -0.02%     
==========================================
  Files          38       38              
  Lines        4082     4080       -2     
==========================================
- Hits         3099     3097       -2     
  Misses        983      983              

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

@tsalo tsalo requested a review from Copilot May 28, 2025 18:46
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes the out_basename input for the BASIL CBF interface and updates the code to use the Nipype node’s working directory for all outputs.

  • Drop out_basename from the interface and workflow connections
  • Switch all output paths in _list_outputs to os.path.abspath of relative subpaths

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
aslprep/workflows/asl/cbf.py Remove the mapping of out_basename in the workflow
aslprep/interfaces/cbf.py Delete the out_basename input and update output specs to use cwd
Comments suppressed due to low confidence (4)

aslprep/interfaces/cbf.py:886

  • The helper _gen_outfilename still references out_file which is no longer defined after removing out_basename. Consider removing or updating this method to prevent a NameError when it's called.
return os.path.abspath(out_file)

aslprep/interfaces/cbf.py:891

  • With output directories now implicitly using the node’s working directory, add or update tests to verify that these absolute paths resolve correctly and that outputs are written under the expected subfolders.
outputs['mean_cbf_basil'] = os.path.abspath('native_space/perfusion_calib.nii.gz')

aslprep/workflows/asl/cbf.py:557

  • [nitpick] Since out_basename has been removed and the working directory is now used, consider adding a comment or updating the workflow docstring to clarify that all BASIL outputs will be written into the node’s working directory.
                (('m0_file', _getfiledir), 'out_basename'),

aslprep/interfaces/cbf.py:893

  • [nitpick] The multi-line os.path.abspath calls have inconsistent indentation and a trailing comma on a single argument, which can be confusing. For readability, either inline the call or align the continued string literal under the opening parenthesis.
outputs['mean_cbf_gm_basil'] = os.path.abspath(

@tsalo tsalo requested a review from mattcieslak May 28, 2025 19:45
@tsalo tsalo merged commit f80cf44 into main May 29, 2025
26 checks passed
@tsalo tsalo deleted the basil-out-dir branch May 29, 2025 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BASIL-CBF interface writes outputs to wrong folder

3 participants