Skip to content

FIX: Detecting and apropriately warning about unconnected duplicatlely nodes #2163

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 11, 2017

Conversation

TheChymera
Copy link
Collaborator

If the workflow contained an unconnected node with a duplicate name, the check failed with a nondescript IndexError, which required the user to either manually re-check their entire connections list for any errors at all, or hack the nipype code with print calls to find out which node is triggering the exception.

Now, the check gives the correct name duplication error and the name of the node, even if the node has no lineage.

@TheChymera TheChymera force-pushed the checks branch 2 times, most recently from e54f020 to 0af73d0 Compare August 22, 2017 18:45
@oesteban
Copy link
Contributor

Could you write a unit test for this? That'd be very much appreciated :)

@codecov-io
Copy link

Codecov Report

Merging #2163 into master will decrease coverage by <.01%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2163      +/-   ##
==========================================
- Coverage   72.25%   72.25%   -0.01%     
==========================================
  Files        1168     1168              
  Lines       58405    58409       +4     
  Branches     8400     8400              
==========================================
  Hits        42203    42203              
- Misses      14868    14872       +4     
  Partials     1334     1334
Flag Coverage Δ
#smoketests 72.25% <50%> (-0.01%) ⬇️
#unittests 69.91% <50%> (-0.01%) ⬇️
Impacted Files Coverage Δ
nipype/pipeline/engine/workflows.py 76.05% <50%> (-0.18%) ⬇️
nipype/interfaces/base.py 83.3% <0%> (-0.19%) ⬇️

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 32e724c...0af73d0. Read the comment docs.

@codecov-io
Copy link

codecov-io commented Aug 23, 2017

Codecov Report

Merging #2163 into master will increase coverage by <.01%.
The diff coverage is 96.15%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2163      +/-   ##
==========================================
+ Coverage   72.25%   72.26%   +<.01%     
==========================================
  Files        1168     1169       +1     
  Lines       58405    58429      +24     
  Branches     8400     8400              
==========================================
+ Hits        42203    42221      +18     
- Misses      14868    14873       +5     
- Partials     1334     1335       +1
Flag Coverage Δ
#smoketests 72.26% <96.15%> (ø) ⬆️
#unittests 69.92% <96.15%> (ø) ⬆️
Impacted Files Coverage Δ
nipype/pipeline/engine/tests/test_workflows.py 100% <100%> (ø)
nipype/pipeline/engine/workflows.py 76.39% <83.33%> (+0.16%) ⬆️
nipype/interfaces/base.py 82.94% <0%> (-0.55%) ⬇️

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 32e724c...e7b9a10. Read the comment docs.

@TheChymera
Copy link
Collaborator Author

@oesteban added :)

@TheChymera
Copy link
Collaborator Author

TheChymera commented Sep 8, 2017

@oesteban @satra ping?

@satra satra merged commit 462a77f into nipy:master Sep 11, 2017
@TheChymera TheChymera deleted the checks branch September 11, 2017 18:56
@satra satra added this to the 0.14.0 milestone Oct 20, 2017
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