Skip to content

FIX: Return afni.Qwarp outputs as absolute paths #2705

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

Merged
merged 1 commit into from
Sep 19, 2018

Conversation

effigies
Copy link
Member

Summary

afni.QwarpPlusMinus provides a default out_file as a relative path, causing afni.Qwarp._list_outputs to return all outputs as relative. This PR always gets the absolute path of the prefix directory, and places all outputs there.

Follow-up to #2674.
Reported in nipreps/fmriprep#1287.

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

  • Fix _gen_filename, which used a missing input
  • Locate all specified outputs in the absolute path of the directory implied by -prefix.

Acknowledgment

  • (Mandatory) I acknowledge that this contribution will be available under the Apache 2 license.

@effigies
Copy link
Member Author

Resolves issues on our end. Anybody up for a quick review?

@satra
Copy link
Member

satra commented Sep 19, 2018

@effigies - the code looks fine - but perhaps an afni person (@rmarkello) can take a quick look?

@codecov-io
Copy link

Codecov Report

Merging #2705 into master will increase coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2705      +/-   ##
==========================================
+ Coverage    67.6%   67.61%   +<.01%     
==========================================
  Files         340      340              
  Lines       43152    43153       +1     
  Branches     5351     5351              
==========================================
+ Hits        29174    29176       +2     
+ Misses      13276    13275       -1     
  Partials      702      702
Flag Coverage Δ
#smoketests 50.57% <ø> (ø) ⬆️
#unittests 65.1% <0%> (+0.02%) ⬆️
Impacted Files Coverage Δ
nipype/interfaces/afni/preprocess.py 81.39% <0%> (-0.1%) ⬇️
nipype/utils/profiler.py 45.4% <0%> (+1.14%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 82121e1...b7773de. Read the comment docs.

@rmarkello
Copy link
Collaborator

Looks alright to me! Good catch on the source_file name change, too. 👍

@effigies
Copy link
Member Author

Thanks for the reviews, @rmarkello and @satra!

I'll merge when the CI is idle or we have a batch of mergeable PRs.

@effigies effigies merged commit 0b4d83a into nipy:master Sep 19, 2018
@effigies effigies deleted the fix/qwarpplusminus branch September 19, 2018 20:25
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.

4 participants