Skip to content

[ENH] Minor Xvfb and MultiProcPlugin cleanups #2211

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
Oct 6, 2017

Conversation

effigies
Copy link
Member

@effigies effigies commented Oct 5, 2017

Quick follow-up to #2200, #2203, #2208.

Changes proposed in this pull request

  • Use relative import to ensure the correct nipype's config is being imported
  • Remember MultiProcPlugin._send_proc_to_workers stats between calls to reduce noise

@mgxd
Copy link
Member

mgxd commented Oct 5, 2017

I was just about to comment about _send_procs_to_workers but you beat me to the punch :)

@effigies
Copy link
Member Author

effigies commented Oct 5, 2017

Added a bunch of make specs updates that got missed in #2200.

@mgxd Let me know if there are any other cleanups that should go in with this.

@mgxd
Copy link
Member

mgxd commented Oct 5, 2017

LGTM - one little thing that could be useful is supplementing the information when running a Function interface here.

Currently, this is output in the log

171005-16:29:28,86 workflow INFO:
	 Running a "Function" interface.
171005-16:29:28,113 workflow INFO:
	 Running a "Function" interface.

which is nice, but maybe we can add the node/function name in this case to get a better understanding?

This could also be addressed in an add-on PR

@effigies
Copy link
Member Author

effigies commented Oct 5, 2017

What do you think of 5b367f2?

@oesteban
Copy link
Contributor

oesteban commented Oct 5, 2017

Hopefully this will polish those little things here and there. Thanks a lot @effigies

@effigies
Copy link
Member Author

effigies commented Oct 6, 2017

Reviewed Oscar's changes. I'm +1 to merge if tests pass.

@effigies effigies force-pushed the fix/xvfb_and_noise branch from cfe9f5b to ef7ccb0 Compare October 6, 2017 02:25
@codecov-io
Copy link

Codecov Report

Merging #2211 into master will increase coverage by <.01%.
The diff coverage is 85.1%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #2211      +/-   ##
=========================================
+ Coverage    72.3%   72.3%   +<.01%     
=========================================
  Files        1178    1178              
  Lines       58736   58768      +32     
  Branches     8456    8458       +2     
=========================================
+ Hits        42468   42494      +26     
- Misses      14896   14901       +5     
- Partials     1372    1373       +1
Flag Coverage Δ
#smoketests 72.3% <85.1%> (ø) ⬆️
#unittests 69.85% <82.97%> (ø) ⬆️
Impacted Files Coverage Δ
...ipype/interfaces/afni/tests/test_auto_ABoverlap.py 85.71% <ø> (ø) ⬆️
nipype/interfaces/afni/tests/test_auto_Despike.py 85.71% <ø> (ø) ⬆️
nipype/interfaces/afni/tests/test_auto_Qwarp.py 85.71% <ø> (ø) ⬆️
nipype/interfaces/afni/tests/test_auto_Unifize.py 85.71% <ø> (ø) ⬆️
...ype/interfaces/afni/tests/test_auto_AFNICommand.py 100% <ø> (ø) ⬆️
nipype/interfaces/afni/tests/test_auto_Copy.py 85.71% <ø> (ø) ⬆️
nipype/interfaces/afni/tests/test_auto_TNorm.py 85.71% <ø> (ø) ⬆️
nipype/interfaces/afni/tests/test_auto_ECM.py 85.71% <ø> (ø) ⬆️
nipype/interfaces/afni/tests/test_auto_Bandpass.py 85.71% <ø> (ø) ⬆️
nipype/interfaces/afni/tests/test_auto_Merge.py 85.71% <ø> (ø) ⬆️
... and 50 more

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 4d08138...ef7ccb0. Read the comment docs.

@codecov-io
Copy link

codecov-io commented Oct 6, 2017

Codecov Report

Merging #2211 into master will not change coverage.
The diff coverage is 85.1%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #2211   +/-   ##
======================================
  Coverage    72.3%   72.3%           
======================================
  Files        1178    1178           
  Lines       58768   58768           
  Branches     8458    8458           
======================================
  Hits        42494   42494           
  Misses      14901   14901           
  Partials     1373    1373
Flag Coverage Δ
#smoketests 72.3% <85.1%> (ø) ⬆️
#unittests 69.85% <82.97%> (ø) ⬆️
Impacted Files Coverage Δ
nipype/interfaces/afni/tests/test_auto_Qwarp.py 85.71% <ø> (ø) ⬆️
nipype/interfaces/afni/tests/test_auto_Detrend.py 85.71% <ø> (ø) ⬆️
nipype/interfaces/afni/tests/test_auto_Axialize.py 85.71% <ø> (ø) ⬆️
nipype/interfaces/afni/tests/test_auto_TCorrMap.py 85.71% <ø> (ø) ⬆️
nipype/interfaces/afni/tests/test_auto_Unifize.py 85.71% <ø> (ø) ⬆️
...ype/interfaces/afni/tests/test_auto_AFNICommand.py 100% <ø> (ø) ⬆️
nipype/interfaces/afni/tests/test_auto_Edge3.py 85.71% <ø> (ø) ⬆️
nipype/interfaces/afni/tests/test_auto_Despike.py 85.71% <ø> (ø) ⬆️
...ipype/interfaces/afni/tests/test_auto_ABoverlap.py 85.71% <ø> (ø) ⬆️
nipype/interfaces/afni/tests/test_auto_SVMTrain.py 85.71% <ø> (ø) ⬆️
... and 107 more

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 4d08138...ef7ccb0. Read the comment docs.

@effigies effigies merged commit 6cda3ef into nipy:master Oct 6, 2017
@effigies effigies deleted the fix/xvfb_and_noise branch October 6, 2017 11:19
@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.

5 participants