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

Fixed bug for work dirs longer than 255 characters, fixes #2061 #3495

Merged
merged 4 commits into from
Sep 5, 2022

Conversation

prioux
Copy link
Contributor

@prioux prioux commented Jul 21, 2022

Summary

This change is fully backwards compatible and will
not affect existing pipelines, but it will prevent
the pipeline engine from crashing when some parameters
are long and it was trying to create subdirectories
longer than 255 characters (a typical unix limit).

Fixes #2061 .

@prioux prioux force-pushed the fix_255_chars_dirs branch from 83a1536 to ec43408 Compare July 21, 2022 20:03
This change is fully backwards compatible and will
not affect existing pipelines, but it will prevent
the pipeline engine from crashing when some parameters
are long and it was trying to create subdirectories
longer than 255 characters (a typical unix limit).
@prioux prioux force-pushed the fix_255_chars_dirs branch from ec43408 to 99aad8a Compare July 21, 2022 20:04
@codecov
Copy link

codecov bot commented Jul 21, 2022

Codecov Report

Merging #3495 (0094bed) into master (ff8e514) will increase coverage by 1.69%.
The diff coverage is 100.00%.

❗ Current head 0094bed differs from pull request most recent head 97c05fb. Consider uploading reports for the commit 97c05fb to get more accurate results

@@            Coverage Diff             @@
##           master    #3495      +/-   ##
==========================================
+ Coverage   63.62%   65.31%   +1.69%     
==========================================
  Files         309      309              
  Lines       40855    40965     +110     
  Branches     5379     5384       +5     
==========================================
+ Hits        25995    26758     +763     
+ Misses      13835    13134     -701     
- Partials     1025     1073      +48     
Flag Coverage Δ
unittests 65.03% <100.00%> (?)

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

Impacted Files Coverage Δ
nipype/pipeline/engine/nodes.py 79.04% <100.00%> (-0.04%) ⬇️
nipype/pipeline/engine/utils.py 75.20% <100.00%> (+1.37%) ⬆️
nipype/interfaces/afni/preprocess.py 81.55% <0.00%> (+0.54%) ⬆️
nipype/interfaces/spm/preprocess.py 50.28% <0.00%> (+0.84%) ⬆️
nipype/interfaces/freesurfer/model.py 63.73% <0.00%> (+1.09%) ⬆️
nipype/utils/subprocess.py 86.66% <0.00%> (+1.48%) ⬆️
nipype/interfaces/freesurfer/preprocess.py 66.04% <0.00%> (+1.65%) ⬆️
nipype/interfaces/base/core.py 87.96% <0.00%> (+2.33%) ⬆️
nipype/interfaces/base/specs.py 88.77% <0.00%> (+2.55%) ⬆️
... and 23 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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.

This seems pretty safe.

Co-authored-by: Chris Markiewicz <effigies@gmail.com>
Copy link
Contributor Author

@prioux prioux left a comment

Choose a reason for hiding this comment

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

See comments

prioux and others added 2 commits August 1, 2022 13:45
@effigies effigies merged commit af3f8df into nipy:master Sep 5, 2022
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.

Maximum filename length
2 participants