Testing 5.0.10

[TomMaullin]

What does this PR do?

This is a small PR which fixing a bug discovered when testing the current exporter against data from FSL 5.0.10. The bug was that the cluster table header for F contrasts can have either 5 or 7 columns depending on the type of analysis/contrast being recorded. Previously, only the case where 5 columns were present had been tested.

Link to relevant issues

PR submission checklist

• [x] All tests in the test suite pass
• [x] Tick when PR is ready for review (i.e. not work-in-progress)

[pep8speaks]

Hello @TomMaullin! Thanks for updating the PR.

• In the file nidmfsl/fsl_exporter/fsl_exporter.py, following are the PEP8 issues :

  Line 138:34: W605 invalid escape sequence '\d' Line 273:22: W605 invalid escape sequence ')' Line 279:22: W605 invalid escape sequence '.' Line 279:24: W605 invalid escape sequence '\d' Line 279:27: W605 invalid escape sequence ')' Line 279:41: W605 invalid escape sequence '\d' Line 303:22: W605 invalid escape sequence '.' Line 303:24: W605 invalid escape sequence '\d' Line 303:27: W605 invalid escape sequence ')' Line 303:41: W605 invalid escape sequence '\d' Line 452:17: W605 invalid escape sequence '\d' Line 789:23: W605 invalid escape sequence '\d' Line 789:26: W605 invalid escape sequence ')' Line 794:34: W605 invalid escape sequence '\d' Line 794:38: W605 invalid escape sequence ')' Line 883:18: W605 invalid escape sequence ')' Line 883:29: W605 invalid escape sequence '\d' Line 943:18: W605 invalid escape sequence ')' Line 943:31: W605 invalid escape sequence '\d' Line 951:14: W605 invalid escape sequence ')' Line 951:27: W605 invalid escape sequence '\d' Line 1086:24: W605 invalid escape sequence '\d' Line 1563:1: W293 blank line contains whitespace Line 1569:29: E225 missing whitespace around operator Line 1577:21: E303 too many blank lines (2)

Comment last updated on September 26, 2018 at 10:35 Hours UTC

[TomMaullin]

Hi @cmaumet ,

Following one small bug fix, I have managed to make sure that all NIDM-Results packs on the fsl_5.0.10 branch export. However, the cluster coordinates have come out slightly differently as we now use python code to derive them instead of the call to cluster. It can be seen in the logs however that they are very similar.

My suggestion would be to update the ground truth in fsl_5.0.10 to match what has been output here. What are your thoughts on this?

[cmaumet]

Hi @TomMaullin! Thanks for tracking this down! Updating the coordinates in the ground truth sounds fine but I thought we had already done it in https://github.com/incf-nidash/nidmresults-examples/pull/105?

[TomMaullin]

Hi @cmaumet , if I remember correctly we did this for the data generated by FSL 5.0.8. As only the group level nidmresults packs generated by FSL 5.0.8 had recorded cluster xyz coordinates, I suspect we only updated the ground truth for the group level analyses and not the subject level.

[cmaumet]

I had another look and the only modified ground truth is fsl_full_example001 which is a single-subject analysis https://github.com/incf-nidash/nidmresults-examples/pull/105/files. But thinking about it, all we might have to do is merge master into the 5.0.10 branch so that those changes to the ground truth are applied. Let me try!

[cmaumet]

@TomMaullin: I've just rebased https://github.com/incf-nidash/nidmresults-examples/pull/103 on master. Can you try running the test suite again?

[TomMaullin]

@cmaumet Ah yes apologies you're completely right! All tests pass! Is this branch okay to merge?

[cmaumet]

Great! Yes, let's merge! Can you just shortly document the small bug fix you made? Ideally, if you can edit the top comment in this PR that would be great! (It's easier to retrieve later on if the top comment is up to date with what was finally put in the PR).