Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

[WIP] FIX - AFNI Zeropad sets out_file #3641

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

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

Conversation

Copy link

@zachlindsey zachlindsey commented Mar 20, 2024

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 •
edited
Loading

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.

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
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):

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.

Copy link
Member

@effigies effigies left a comment

Choose a reason for hiding this comment

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

Left a suggestion a while back. Marking as requested changes so I can easily see the status. Please re-request review if/when you update this.

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

@effigies effigies effigies requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AFNI Zeropad out_file undefined

AltStyle によって変換されたページ (->オリジナル) /