incf-nidash / nidmresults-fsl

A python library to export FSL's feat results to NIDM-Results
http://nidm.nidash.org/specs/nidm-results.html
MIT License
3 stars 11 forks source link

Compatibility with nidmresults 2.0.0-rc1 #124

Closed TomMaullin closed 6 years ago

TomMaullin commented 6 years ago

This PR updates NIDMResults-FSL to work with the latest update of NIDMResults (found here).

pep8speaks commented 6 years ago

Hello @TomMaullin! Thanks for updating the PR.

Line 29:1: E402 module level import not at top of file Line 30:1: E402 module level import not at top of file Line 31:1: E402 module level import not at top of file Line 32:1: E402 module level import not at top of file Line 33:1: E402 module level import not at top of file Line 34:1: E402 module level import not at top of file Line 87:9: E722 do not use bare except' Line 147:9: E722 do not use bare except'

Line 15:12: E251 unexpected spaces around keyword / parameter equals Line 15:14: E251 unexpected spaces around keyword / parameter equals Line 16:13: E251 unexpected spaces around keyword / parameter equals Line 16:15: E251 unexpected spaces around keyword / parameter equals

Line 129:21: E115 expected an indented block (comment) Line 130:21: E115 expected an indented block (comment) Line 131:21: E115 expected an indented block (comment) Line 140:33: E122 continuation line missing indentation or outdented Line 141:33: E122 continuation line missing indentation or outdented

Line 22:1: E402 module level import not at top of file Line 23:1: E402 module level import not at top of file Line 24:1: E402 module level import not at top of file Line 25:1: E402 module level import not at top of file Line 27:1: E402 module level import not at top of file Line 86:1: E305 expected 2 blank lines after class or function definition, found 1

Comment last updated on June 14, 2018 at 07:35 Hours UTC
TomMaullin commented 6 years ago

Hi @cmaumet ,

This branch is now ready for review. The travis tests currently fail as they download the master branch of nidmresults and not the refactor branch that they are designed to work with. I am unsure whether it is worth changing the Travis tests or just waiting until everything has been merged back into the master.

cmaumet commented 6 years ago

HI @TomMaullin. Thanks for these updates! Given that this PR is to check compatibility with nidmresults 2.0, I think it would make sense to update the travis config file to install the 2.0 version (rather than the default from master). Can you update this and then I'll have a look at the code? It should only be a one-line change in the config file, right? Thanks!!

TomMaullin commented 6 years ago

Yes sure, I will do this now!

cmaumet commented 6 years ago

@TomMaullin: As in https://github.com/cmaumet/nidmresults/pull/1, I've added a couple of updates to (hopefully) make the tests pass:

TomMaullin commented 6 years ago

Hi @cmaumet , These changes make sense to me! We just have to remember we have deactivated fsl_con_f_multiple later! If the tests on cmaumet/nidmresults#1 pass, I'm happy to merge if you're happy on merging cmaumet/nidmresults#1!

cmaumet commented 6 years ago

Released on pypi: https://pypi.org/manage/project/nidmfsl/release/2.0.0/.