Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] FIX - AFNI Zeropad sets out_file #3641

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

zachlindsey
Copy link

Summary

Fixes #3640 .

List of changes proposed in this PR (pull-request)

  • Remove name_template from ZeropadInputSpec, since it is useless if it is not a format string and the underlying command doesn't form outputs from the input name.
  • Implement a custom _list_outputs method to set the correct out file.

Copy link

codecov bot commented Mar 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.00%. Comparing base (edf7a4a) to head (bc4af2b).
Report is 12 commits behind head on master.

❗ Current head bc4af2b differs from pull request most recent head 6b81d26. Consider uploading reports for the commit 6b81d26 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #3641       +/-   ##
==========================================
- Coverage   63.44%       0   -63.45%     
==========================================
  Files         308       0      -308     
  Lines       40887       0    -40887     
  Branches     5655       0     -5655     
==========================================
- Hits        25942       0    -25942     
+ Misses      13909       0    -13909     
+ Partials     1036       0     -1036     

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

@zachlindsey
Copy link
Author

After trying out this fix for my workflow, I've realized that out_file needs to be an absolute path! Will fix.

@zachlindsey zachlindsey changed the title FIX - AFNI Zeropad sets out_file [WIP] FIX - AFNI Zeropad sets out_file Mar 21, 2024
@zachlindsey zachlindsey reopened this Mar 21, 2024
@zachlindsey zachlindsey reopened this Mar 21, 2024
Comment on lines +1 to +29
from pathlib import Path
from nipype import Node
from nipype.interfaces.afni import Zeropad
from nipype.testing.fixtures import create_files_in_directory


def test_zeropad_handles_outfile_default(create_files_in_directory):
filelist, outdir = create_files_in_directory
zp = Zeropad(I=1)
zp.inputs.in_files = filelist[0]

result = zp.run()

assert (Path(outdir) / "zeropad+tlrc.BRIK").exists()
assert Path(result.outputs.out_file).name == "zeropad+tlrc.BRIK"


def test_zeropad_handles_outfile_specified_nii_gz(create_files_in_directory):
filelist, outdir = create_files_in_directory
zp = Zeropad(I=1, out_file="padded.nii.gz")
zp.inputs.in_files = filelist[0]

result = zp.run()

assert (Path(outdir) / "padded.nii.gz").exists()
assert Path(result.outputs.out_file).name == "padded.nii.gz"


def test_zeropad_keeps_file_after_node_run(create_files_in_directory):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
from pathlib import Path
from nipype import Node
from nipype.interfaces.afni import Zeropad
from nipype.testing.fixtures import create_files_in_directory
def test_zeropad_handles_outfile_default(create_files_in_directory):
filelist, outdir = create_files_in_directory
zp = Zeropad(I=1)
zp.inputs.in_files = filelist[0]
result = zp.run()
assert (Path(outdir) / "zeropad+tlrc.BRIK").exists()
assert Path(result.outputs.out_file).name == "zeropad+tlrc.BRIK"
def test_zeropad_handles_outfile_specified_nii_gz(create_files_in_directory):
filelist, outdir = create_files_in_directory
zp = Zeropad(I=1, out_file="padded.nii.gz")
zp.inputs.in_files = filelist[0]
result = zp.run()
assert (Path(outdir) / "padded.nii.gz").exists()
assert Path(result.outputs.out_file).name == "padded.nii.gz"
def test_zeropad_keeps_file_after_node_run(create_files_in_directory):
from pathlib import Path
from shutil import which
import pytest
from nipype import Node
from nipype.interfaces.afni import Zeropad
from nipype.testing.fixtures import create_files_in_directory
needs_3dZeropad = pytest.mark.skipif(
not which('3dZeropad'),
reason='Needs AFNI 3dZeropad installed'
)
@needs_3dZeropad
def test_zeropad_handles_outfile_default(create_files_in_directory):
filelist, outdir = create_files_in_directory
zp = Zeropad(I=1)
zp.inputs.in_files = filelist[0]
result = zp.run()
assert (Path(outdir) / "zeropad+tlrc.BRIK").exists()
assert Path(result.outputs.out_file).name == "zeropad+tlrc.BRIK"
@needs_3dZeropad
def test_zeropad_handles_outfile_specified_nii_gz(create_files_in_directory):
filelist, outdir = create_files_in_directory
zp = Zeropad(I=1, out_file="padded.nii.gz")
zp.inputs.in_files = filelist[0]
result = zp.run()
assert (Path(outdir) / "padded.nii.gz").exists()
assert Path(result.outputs.out_file).name == "padded.nii.gz"
@needs_3dZeropad
def test_zeropad_keeps_file_after_node_run(create_files_in_directory):

@effigies
Copy link
Member

Made a suggestion. Haven't tested it, but if you commit and run locally, those tests should pass. They'll skip on CI.

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.

AFNI Zeropad out_file undefined
2 participants