Skip to content

FIX: Correctly connect JoinNodes in nested iterables #2597

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 4 commits into from
May 26, 2018

Conversation

effigies
Copy link
Member

Fixes #2578
Follow-up to #2479

Changes proposed in this pull request

  • Narrow node name check to ensure JoinNodes aren't excluded incorrectly
  • Add test

@effigies effigies added this to the 1.0.4 milestone May 25, 2018
@effigies
Copy link
Member Author

@tclose Can you verify that this resolves your issue?

@codecov-io
Copy link

codecov-io commented May 25, 2018

Codecov Report

Merging #2597 into master will decrease coverage by 0.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #2597      +/-   ##
=========================================
- Coverage   67.62%   67.6%   -0.02%     
=========================================
  Files         336     336              
  Lines       42711   42711              
  Branches     5278    5278              
=========================================
- Hits        28883   28875       -8     
- Misses      13151   13158       +7     
- Partials      677     678       +1
Flag Coverage Δ
#smoketests 50.74% <0%> (ø) ⬆️
#unittests 65.08% <0%> (-0.02%) ⬇️
Impacted Files Coverage Δ
nipype/pipeline/engine/utils.py 82.24% <0%> (-0.23%) ⬇️
nipype/pipeline/plugins/multiproc.py 79.76% <0%> (-2.39%) ⬇️
nipype/utils/profiler.py 43.1% <0%> (-1.15%) ⬇️

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 9eaa2a3...6c9bb4d. Read the comment docs.

@effigies
Copy link
Member Author

@satra Would appreciate your review of this. I think it's straightforward, but you might spot an edge case I've missed.

@tclose
Copy link
Contributor

tclose commented May 26, 2018

Thanks Chris, this change fixes the problem.

@effigies
Copy link
Member Author

Great! Glad to hear it. We should have a release out that includes this fix on Tuesday.

@effigies effigies merged commit ac84ef1 into nipy:master May 26, 2018
@effigies effigies deleted the fix/joinnode branch May 26, 2018 01:43
@tclose
Copy link
Contributor

tclose commented May 26, 2018

Perfect!

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.

3 participants