We've added two tests; the first, a unit test, that requires themock library. (which is why it's currently failing). If this is not desirable, I'll take it out. If it is, where should I add themock dependency, inrequirements.txt ortravis.yml?
The second test does a save->load type acceptance test.

Thanks for doing this. Would you mind posting to the mailing list to ask for opinions about relying on the 'mock' library?

Thanks. Would you mind adding mock to thedev-requirements.txt file as well. Could yougrep fornose in the docs section, to update the installation / testing instructions?

I added a check for "mock" when running tests vianibabel.test() :JasperVanDenBosch#1

Use the machinery intest_scripts.py to find the executable?

I completely overlooked that.. but the downside of that scriptrunner is that we still wouldn't be able to test the separate functions / import the code.

Long term I think ideally we'd refactor most of the logic out of theparrec2nii executable and into a regular module, which is more straightforward to test. For now I added a list of possible binary directories to look for parrec2nii.

Coverage increased (+0.008%) to 96.251% when pullinge23dafc on ilogue:qform-provided-then-qform-code-2 intoe37cb5d on nipy:master.

Coverage increased (+0.008%) to 96.251% when pullinge23dafc on ilogue:qform-provided-then-qform-code-2 intoe37cb5d on nipy:master.

Current coverage is 93.81% (diff: 51.26%)

Merging#478 intomaster will decrease coverage by0.46%

@@             master       #478   diff @@==========================================  Files           161        163     +2     Lines         21321      21555   +234     Methods           0          0            Messages          0          0            Branches       2284       2315    +31   ==========================================+ Hits          20100      20221   +121- Misses          801        902   +101- Partials        420        432    +12

Powered byCodecov. Last updatee37cb5d...aa184ed

How aboutJasperVanDenBosch#2 ?

Looks good, thanks! I'll check back if there's any failing tests.

Coverage decreased (-0.4%) to 95.815% when pullingaa184ed on ilogue:qform-provided-then-qform-code-2 intoe37cb5d on nipy:master.

Thanks. I think coverage is down quite a bit, but I believe that's because the parrec2nii command code wasn't covered well, and we are now seeing that in the coverage report, so no not the fault of this PR.

Current PR is fine with me.

Any other suggestions?

No further suggestions! Thanks

On Aug 11, 2016 2:49 PM, "Matthew Brett"notifications@github.com wrote:

Thanks. I think coverage is down quite a bit, but I believe that's because
the parrec2nii command code wasn't covered well, and we are now seeing that
in the coverage report, so no not the fault of this PR.

Current PR is fine with me.

Any other suggestions?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#478 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABcEjIWg2Nzz6Z3woX-H0cyLDQ2nSyfgks5qe5kDgaJpZM4JglbJ
.

Actually, sorry, now I think if it - shouldn't thesform andqform codes both be 1 here? In that they indicate the scanner coordinates?@grlee77 - what do you think? Will this cause problems for you?

hi matthew, because we are dealing with un-transformed data (pre-alignment with standard brains etc), both the q and sforms (and codes) should be the same whichever is decided. as i understand it, sform is diverged from qform when transformed into standard space and qform is kept to reference path to original subj space. see
https://github.com/stnava/ANTs/wiki/How-does-ANTs-handle-qform-and-sform-in-NIFTI-1-images%3F
Currently only sform is saved and sform code=2 in parrec2nii. see header dumps in my orig issues posthttps://github.com/nipy/nibabel/issues/463
you are right both codes should =1 if --origin='scanner' is default and no other transformation applied.
since arbitrarily sform code=2 when nibabel saves parrec images, does it change code from 1 to 2 when the parrec are transformed from Philips PLS to RAS+? technically that is a transformation by applying an affine but not an 'alignment'. this may be the default behavior since this exception has not been accounted for.
rather than ferret out this exception or have 2 different codes for the same space, i vote to manually set both sform and qform_code=1 e.g. 'Scanner Anat'. as in lines 220-226 in parrec2nii with my last 2 lines added:
# Make corresponding NIfTI image nimg = nifti1.Nifti1Image(in_data, affine, pr_hdr) nhdr = nimg.header nhdr.set_data_dtype(out_dtype) nhdr.set_slope_inter(slope, intercept) nhdr['qform_code'] = 1 nhdr['sform_code'] = 1
then both are saved identically and both =1.

qform / sform code of 1 is fine with me, but it is a small API break. If there is some software that behaves differently with sform codes of 1 and 2 then something would change, but I doubt that would happen. OK to take the risk?

I am fine with setting both to 1. (The previous behavior was sform=2, qform=0?)

Right

OK - I'll merge this as-is, then do a PR for the code changes.