Skip to content

BF: Run make specs with CWD in PYTHONPATH #1813

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 3 commits into from
Feb 23, 2017
Merged

Conversation

effigies
Copy link
Member

Not an expert at Makefiles, so this may not be the best way to go about this, but it does seem to set the PYTHONPATH so that make specs is run on the working directory, rather than an installed nipype.

Fixes #1348.

@effigies
Copy link
Member Author

Might as well run make specs and include the updated tests with this patch.

@codecov-io
Copy link

codecov-io commented Feb 14, 2017

Codecov Report

Merging #1813 into master will increase coverage by 0.38%.
The diff coverage is 98.63%.

@@            Coverage Diff             @@
##           master    #1813      +/-   ##
==========================================
+ Coverage   72.71%   73.09%   +0.38%     
==========================================
  Files        1059     1064       +5     
  Lines       52550    53316     +766     
==========================================
+ Hits        38212    38972     +760     
- Misses      14338    14344       +6
Flag Coverage Δ
#unittests 73.09% <98.63%> (+0.38%)
Impacted Files Coverage Δ
...aces/freesurfer/tests/test_auto_FSCommandOpenMP.py 100% <100%> (ø)
...ces/ants/tests/test_auto_AverageAffineTransform.py 92.85% <100%> (+0.54%)
.../interfaces/freesurfer/tests/test_auto_Resample.py 92.85% <100%> (+0.54%)
...rfaces/freesurfer/tests/test_auto_SurfaceSmooth.py 92.85% <100%> (+0.54%)
nipype/interfaces/dipy/tests/test_auto_CSD.py 92.85% <100%> (+0.54%)
...interfaces/freesurfer/tests/test_auto_SegmentWM.py 92.85% <100%> (+0.54%)
...ype/interfaces/ants/tests/test_auto_JointFusion.py 92.85% <100%> (+0.54%)
...faces/freesurfer/tests/test_auto_RobustRegister.py 92.85% <100%> (+0.54%)
nipype/interfaces/fsl/tests/test_auto_B0Calc.py 92.85% <100%> (+0.54%)
...ipype/interfaces/brainsuite/tests/test_auto_Bse.py 92.85% <100%> (+0.54%)
... and 697 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 20f0444...f8fb369. Read the comment docs.

@effigies effigies force-pushed the issue1348 branch 4 times, most recently from 0ecb22a to af5e99a Compare February 22, 2017 01:10
@effigies
Copy link
Member Author

@satra Do you want to have a quick look? This is just a quick fixup to make make specs work properly for devs with an installed copy of nipype and/or working in Python 3.

@@ -38,9 +38,9 @@ def test_Eddy_inputs():
mandatory=True,
),
in_topup_fieldcoef=dict(argstr='--topup=%s',
requires=['in_topup_movpar'],
requires=[u'in_topup_movpar'],
Copy link
Member

Choose a reason for hiding this comment

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

the primary reason we were using python 2 was because of unicode issues. however, if we include unicode_literals at the top of template, we can generate the same tests from either py2 or py3 by modifying the test generator a bit.

the main point of these auto-tests were to create an easy visual check for changes in input/outputspec api and to instantiate each object in the absence of consistent doctests. the former we can capture through a good PR review. the latter would require some work to go through all interfaces. so if we can achieve these two things somehow, it would be good.

Copy link
Member Author

@effigies effigies Feb 22, 2017

Choose a reason for hiding this comment

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

7ff705f makes py2/py3 behavior equivalent. I'm not entirely sure what you're suggesting for instantiating each object... Does the current system not do that?

@effigies effigies force-pushed the issue1348 branch 2 times, most recently from 813ccf7 to 8f69895 Compare February 22, 2017 17:10
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