Skip to content

FIX: BET raising "No image files match: ..." with very long file names #3309

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 6 commits into from
Mar 31, 2021

Conversation

ZviBaratz
Copy link
Contributor

Trying to fix #3211.
@satra or @oesteban, I'll try a couple of things and see if it works, but I'm not confident if I completely understood:

  1. make the output filename be local (no absolute path) for generation, but not for list_outputs.

If you could help clarify how exactly this would be best implemented, I would be happy to update the PR and hopefully resolve this issue.

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

@codecov
Copy link

codecov bot commented Mar 4, 2021

Codecov Report

Merging #3309 (489a48d) into master (47fe00b) will increase coverage by 0.00%.
The diff coverage is 77.77%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3309   +/-   ##
=======================================
  Coverage   64.70%   64.70%           
=======================================
  Files         302      302           
  Lines       39869    39875    +6     
  Branches     5288     5289    +1     
=======================================
+ Hits        25796    25801    +5     
- Misses      12984    12985    +1     
  Partials     1089     1089           
Flag Coverage Δ
unittests 64.70% <77.77%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
nipype/interfaces/fsl/preprocess.py 71.50% <77.77%> (+0.09%) ⬆️

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 47fe00b...489a48d. Read the comment docs.

@ZviBaratz
Copy link
Contributor Author

ZviBaratz commented Mar 4, 2021

I changed _gen_outfilename() and _list_outputs() to the best of my understanding, but it doesn't seem to solve the problem (at least not in my use case).

@ZviBaratz ZviBaratz changed the title FIX BET raising "No image files match: ..." with very long file names FIX: BET raising "No image files match: ..." with very long file names Mar 4, 2021
@ZviBaratz
Copy link
Contributor Author

Just for context: I encountered this issue trying to run MRIQC. It seems even running BET manually with the MRIQC workflow's relative work/ paths raises an error.

@satra
Copy link
Member

satra commented Mar 9, 2021

@ZviBaratz - thank you for taking a stab at this. could you please tell me what the BET command (the cmdline that get's executed) looks like before it crashes?

@oesteban
Copy link
Contributor

oesteban commented Mar 9, 2021

@satra #3211 (comment)

@ZviBaratz
Copy link
Contributor Author

@satra This is the command generated by MRIQC when using nipype on this PR's branch:

bet /home/flavus/Projects/mriqc/work/mriqc_wf/anatMRIQC/HeadMaskWorkflow/_in_file_..media..veracrypt1..media..MRI..NIfTI..sub-772..ses-201905310935..anat..sub-772_acq-54363corrected_T1w.nii.gz/fsl_bet/sub-772_acq-54363corrected_T1w_conformed_corrected.nii.gz sub-772_acq-54363corrected_T1w_conformed_corrected_brain.nii.gz -A

For readability's sake:

in_file = "/home/flavus/Projects/mriqc/work/mriqc_wf/anatMRIQC/HeadMaskWorkflow/_in_file_..media..veracrypt1..media..MRI..NIfTI..sub-772..ses-201905310935..anat..sub-772_acq-54363corrected_T1w.nii.gz/fsl_bet/sub-772_acq-54363corrected_T1w_conformed_corrected.nii.gz"
out_file = "sub-772_acq-54363corrected_T1w_conformed_corrected_brain.nii.gz"

Unfortunately, unlike @oesteban case, even trying to run the generated command manually raises the same exception.
I realize my BIDS labels are slightly long, but I would imagine it to be an acceptable use case. What do you think?

@satra
Copy link
Member

satra commented Mar 10, 2021

@ZviBaratz - the command line doesn't quite look right. the issue is that it's still reflecting the relative path. in this particular case you would do the same thing for in_file that you have done for out_file but by overriding the function format_arg

here is an example from FAST, which you can adapt for BET

def _format_arg(self, name, spec, value):

if you do this for in_file, it should reduce the path length.

@ZviBaratz
Copy link
Contributor Author

ZviBaratz commented Mar 10, 2021

@satra Thank you for clarifying this. I implemented what you suggested, and I am happy to report it indeed solves the issue (at least in my use case).

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.

Looks sensible. Need to get CI back up and running, but I don't think we need to hold this up.

@effigies effigies merged commit 6835d5d into nipy:master Mar 31, 2021
@ZviBaratz ZviBaratz deleted the bet branch March 31, 2021 15:12
@effigies effigies added this to the 1.6.1 milestone May 1, 2021
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.

FSL (+6.0) BET chokes with long filenames
4 participants