REF: Update nipype2boutiques script #2894

erinb90 · 2019-02-28T16:15:49Z

Summary

Refactored and improved the nipype2boutiques script that converts Nipype interfaces into Boutiques descriptors. It's still not perfect, and doesn't cover every edge case (might even crash for some interfaces), but it does a lot more than it did before. May update this or make another PR in the future if I find ways to improve it more.

Examples of script output

@glatard @gkiar

Acknowledgment

(Mandatory) I acknowledge that this contribution will be available under the Apache 2 license.

…value for tool-version when interface version is null

…s the same as an input name

…ices extraction, added method to get input type from handler type

… added check for integer types, added method to get description from spec

…flag to take argstr from input spec, added requires-inputs and disables-inputs

…k to deal with undefined output values

…puts don't disable themselves, made default output path template the output id

…ut specs

…with various different inputs to try and generate as many outputs as possible

… numbers, some other minor fixes

…ly custom path for saved file

…aused too many failures)

codecov-io · 2019-02-28T18:33:22Z

Codecov Report

❗ No coverage uploaded for pull request base (master@460c1bb). Click here to learn what that means.
The diff coverage is 50%.

@@            Coverage Diff            @@
##             master    #2894   +/-   ##
=========================================
  Coverage          ?   64.06%           
=========================================
  Files             ?      342           
  Lines             ?    43835           
  Branches          ?     5507           
=========================================
  Hits              ?    28084           
  Misses            ?    14639           
  Partials          ?     1112

Flag	Coverage Δ
#unittests	`64.06% <50%> (?)`

Impacted Files	Coverage Δ
nipype/utils/nipype2boutiques.py	`52.23% <49.83%> (ø)`
nipype/scripts/cli.py	`42.85% <66.66%> (ø)`

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 460c1bb...01879f2. Read the comment docs.

glatard · 2019-02-28T20:22:47Z

Thanks for doing this @erinb90, it looks great to me!

…nd line, added logic to deal with inputs containing name_source and name_template metadata

…to 1.0.0, added logic to deal with 0/1 booleans and tuples, added list separator and flag separator checks

…e args

…ipype2boutiques

mgxd

Hi @erinb90, thanks for the patch - it's a big jump from the previous version supported! I left a few comments (mostly style) - you'll also have to update the nipypecli call

nipype/nipype/scripts/cli.py

Lines 243 to 254 in 232ac23

    
           def boutiques(interface, module, output, ignored_template_inputs, docker_image, 
        
                         docker_index, ignore_template_numbers, verbose): 
        
               """Nipype to Boutiques exporter. 
        
               See Boutiques specification at https://github.com/boutiques/schema. 
        
               """ 
        
               from nipype.utils.nipype2boutiques import generate_boutiques_descriptor 
        
               # Generates JSON string 
        
               json_string = generate_boutiques_descriptor( 
        
                   module, interface, ignored_template_inputs, docker_image, docker_index, 
        
                   verbose, ignore_template_numbers)

mgxd · 2019-03-27T15:13:57Z

nipype/utils/nipype2boutiques.py

+    tool_desc['tags'] = desc_tags
+
+    # Check for positional arguments and reorder command line args if necessary
+    tool_desc['command-line'] = reorder_cmd_line_args(tool_desc['command-line'], interface, ignore_inputs)


rf: line length

@mgxd What should be the max line length?

We should follow pep8 conventions, so 79 characters

mgxd · 2019-03-27T15:14:33Z

nipype/utils/nipype2boutiques.py

+        if verbose:
+            print("-> Descriptor saved to file " + outfile.name)
+
+    print("NOTE: Descriptors produced by this script may not entirely conform to the Nipype interface "


rf: line length

mgxd · 2019-03-27T15:44:13Z

nipype/utils/nipype2boutiques.py

+
+    inp = {}
+
+    if input_number is not None and input_number != 0:  # No need to append a number to the first of a list of compound inputs


Suggested change

if input_number is not None and input_number != 0: # No need to append a number to the first of a list of compound inputs

if input_number: # No need to append a number to the first of a list of compound inputs

mgxd · 2019-03-27T15:16:08Z

nipype/utils/nipype2boutiques.py

+    elif handler_type == "Float":
+        inp['type'] = "Number"
+    elif handler_type == "Bool":
+        if spec.argstr and len(spec.argstr.split("=")) > 1 and (spec.argstr.split("=")[1] == '0' or spec.argstr.split("=")[1] == '1'):


rf: line length

mgxd · 2019-03-27T15:16:39Z

nipype/utils/nipype2boutiques.py

+                    source = inp_spec.name_source[0]
+                else:
+                    source = inp_spec.name_source
+                output['path-template'] = inp_spec.name_template.replace("%s", "[" + source.upper() + "]")


rf: line length

mgxd · 2019-03-27T15:17:25Z

nipype/utils/nipype2boutiques.py

-def get_type_from_spec_info(spec_info):
+def get_boutiques_groups(input_traits):
+    """
+    Returns a list of dictionaries containing Boutiques groups for the mutually exclusive and all-or-none Nipype inputs.


rf: line length

mgxd · 2019-03-27T15:17:47Z

nipype/utils/nipype2boutiques.py

+            continue
+        positional_args.append(item[1])
+
+    return interface_name + " " + " ".join(positional_args) + " " + ((last_arg + " ") if last_arg else "") + " ".join(non_positional_args)


rf: line length

mgxd · 2019-03-27T15:37:17Z

nipype/utils/nipype2boutiques.py

-        tool_desc['docker-index'] = docker_index
+    tool_desc['output-files'] = []
+    tool_desc['groups'] = []
+    tool_desc['tool-version'] = interface.version if interface.version is not None else "1.0.0"


Wouldn't it be better to state no version was provided rather than assume this?

That's what I thought at first, but then I found it looked weird on Zenodo to not have a version number here:

mgxd · 2019-03-27T15:41:27Z

nipype/utils/nipype2boutiques.py

+
+    # Save descriptor to a file
+    if save:
+        path = save_path if save_path is not None else os.path.join(os.getcwd(), interface_name + '.json')


Suggested change

path = save_path if save_path is not None else os.path.join(os.getcwd(), interface_name + '.json')

path = save_path or os.path.join(os.getcwd(), interface_name + '.json')

…output from a Nipype input spec, code style fixes, some refactoring and improvements

mgxd

Great - thanks! To get the tests passing, you'll need to merge with master.

Only other thing I would suggest before merging this in is supplementing / adding a test that checks the boutique output, if you're up to it.

…ipype2boutiques

erinb90 · 2019-03-29T02:41:08Z

nipype/utils/tests/test_nipype2boutiques.py

+                                         author=("Oxford Centre for Functional"
+                                                 " MRI of the Brain (FMRIB)"))
+
+    with open('utils/nipype2boutiques_example.json', 'r',


Hi @mgxd, I want to use this example output file in my test case but am not sure how to reference it (what I have above doesn't work). What should the path be?

mgxd

some suggestions to get stuff working again

mgxd · 2019-03-29T14:16:04Z

nipype/utils/nipype2boutiques_example.json

@@ -0,0 +1,549 @@
+{


this file should go in nipype/testing/data. You can then import the data with

from nipype.testing import example_data example_data('relative/path/from/testing/directory')

mgxd · 2019-03-29T14:22:46Z

nipype/utils/tests/test_nipype2boutiques.py

+                                                 " MRI of the Brain (FMRIB)"))
+
+    with open('utils/nipype2boutiques_example.json', 'r',
+              encoding='utf-8') as desc_file:


encoding will not work on Py2 - though if replace open with from io import open, it should fix it.

I'll just remove encoding, it's not necessary here.

erinb90 · 2019-03-29T21:33:20Z

So my test is failing because on CircleCI the descriptor produced has interface version 5.0.9, while the example file has version 1.0.0 (placeholder when version is not found). When I run nipype2boutiques locally I always see interface.version as None, so how did CircleCI get that version number?

mgxd · 2019-04-01T14:13:56Z

@erinb90 the Circle build has FSL 5.0.9 installed, so the good thing is it's outputting what's expected. To avoid having to constantly reconfigure this test though, I would recommend checking only a subset of fields that will be consistent across environments.

mgxd · 2019-04-09T17:26:34Z

@erinb90 if you sync with current master it'll fix the travis tests

…ipype2boutiques

…se when command line flag includes a 0 or 1 (seen in FNIRT)

satra · 2019-04-18T23:38:33Z

nipype/utils/nipype2boutiques.py

+            inp['type'] = "String"
+        if trait_handler.minlen != 0:
+            inp['min-list-entries'] = trait_handler.minlen
+        if trait_handler.maxlen != six.MAXSIZE:


Suggested change

if trait_handler.maxlen != six.MAXSIZE:

if trait_handler.maxlen != sys.maxsize:

satra · 2019-04-18T23:38:48Z

nipype/utils/nipype2boutiques.py

 import simplejson as json
+import six


we don't use six as a dependency of nipype. this bit should use the relevant pieces from future

Suggested change

import six

I seem to have broken something in the build in removing this import. It fails with:

UnsatisfiableError: The following specifications were found to be in conflict: - matplotlib Use "conda search <package> --info" to see the dependencies for each package.

Do you know where this could come from?

@erinb90 the error is unrelated, and should be fixed if you fetch current master

Sorry for all these CI build inconveniences - it seems everything waited on your PR to break

…ield instead

…ipype2boutiques

erinb90 added 15 commits February 6, 2019 14:50

updated nipype2boutiques to conform to current schema, added default …

7c7d462

…value for tool-version when interface version is null

added a step to make sure output IDs are unique in case output name i…

fc27d01

…s the same as an input name

descriptor gets saved to a file for easier exporting, added value-cho…

e7739e7

…ices extraction, added method to get input type from handler type

added warning message when cannot determine path template for output,…

181c481

… added check for integer types, added method to get description from spec

fixed json file formatting, added save parameter, fixed command-line-…

ce06815

…flag to take argstr from input spec, added requires-inputs and disables-inputs

added logic to deal with range and list inputs, added try except bloc…

c1cfc80

…k to deal with undefined output values

added logic to deal with compound traits, added check to make sure in…

ee4b935

…puts don't disable themselves, made default output path template the output id

put compound inputs into mutex group

ad36016

added method to get all the mutex and all-or-none groups from the inp…

bb260f8

…ut specs

removed input tempvalue stuff and added logic to rerun the interface …

1110b4f

…with various different inputs to try and generate as many outputs as possible

added logic to handle multioutputs and inputs where value choices are…

8e20d2d

… numbers, some other minor fixes

fixed multioutput, added handling of multiinput, added option to supp…

61c836f

…ly custom path for saved file

fixed number input min and max, removed numbers from custom inputs (c…

f4c8867

…aused too many failures)

removed unused things, updated comments

6961f02

fixed small error

f67d04b

erinb90 added 5 commits March 12, 2019 16:20

added author and ignored inputs parameters, removed nipype from comma…

2bf03cf

…nd line, added logic to deal with inputs containing name_source and name_template metadata

fixed code quality issues

d009a88

added option to supply descriptor tags, changed default tool version …

63d96be

…to 1.0.0, added logic to deal with 0/1 booleans and tuples, added list separator and flag separator checks

added method to take into account the order of positional command lin…

1e15b99

…e args

Merge branch 'master' of https://github.com/nipy/nipype into update-n…

54259d8

…ipype2boutiques

mgxd reviewed Mar 27, 2019

View reviewed changes

erinb90 added 2 commits March 27, 2019 17:02

added methods to generate command line flag and generate a boutiques …

f0a11b8

…output from a Nipype input spec, code style fixes, some refactoring and improvements

updated nipypecli, fixed code style

b4b2de0

mgxd approved these changes Mar 28, 2019

View reviewed changes

erinb90 added 3 commits March 28, 2019 20:47

updated nipype2boutiques test

ebe7da7

Merge branch 'master' of https://github.com/nipy/nipype into update-n…

892c03d

…ipype2boutiques

fix test file path

2ff50ed

erinb90 commented Mar 29, 2019

View reviewed changes

mgxd reviewed Mar 29, 2019

View reviewed changes

mgxd added this to the 1.2.0 milestone Mar 29, 2019

fixed test

c828fec

erinb90 added 2 commits April 2, 2019 10:33

changed test to only check a subset of descriptor fields

7e96311

changed test again

0eae216

erinb90 force-pushed the update-nipype2boutiques branch from 8976e35 to 0eae216 Compare April 4, 2019 18:53

erinb90 added 2 commits April 9, 2019 14:09

Merge branch 'master' of https://github.com/nipy/nipype into update-n…

9a17389

…ipype2boutiques

added handling for lists with value choices, fixed logic to handle ca…

52d87a9

…se when command line flag includes a 0 or 1 (seen in FNIRT)

satra requested changes Apr 18, 2019

View reviewed changes

removed six, changed all-or-none groups to go under requires-inputs f…

5c0684d

…ield instead

effigies modified the milestones: 1.2.0, 1.2.1 May 8, 2019

Merge branch 'master' of https://github.com/nipy/nipype into update-n…

01879f2

…ipype2boutiques

satra merged commit c14f24e into nipy:master Aug 13, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

REF: Update nipype2boutiques script #2894

REF: Update nipype2boutiques script #2894

erinb90 commented Feb 28, 2019

codecov-io commented Feb 28, 2019 •

edited

Loading

glatard commented Feb 28, 2019

mgxd left a comment

mgxd Mar 27, 2019

erinb90 Mar 27, 2019

mgxd Mar 28, 2019

mgxd Mar 27, 2019

mgxd Mar 27, 2019

mgxd Mar 27, 2019

mgxd Mar 27, 2019

mgxd Mar 27, 2019

mgxd Mar 27, 2019

mgxd Mar 27, 2019

erinb90 Mar 27, 2019

mgxd Mar 27, 2019

mgxd left a comment

erinb90 Mar 29, 2019

mgxd left a comment

mgxd Mar 29, 2019

mgxd Mar 29, 2019

erinb90 Mar 29, 2019

erinb90 commented Mar 29, 2019

mgxd commented Apr 1, 2019

mgxd commented Apr 9, 2019

satra Apr 18, 2019

satra Apr 18, 2019

erinb90 Apr 24, 2019

mgxd May 2, 2019

	def boutiques(interface, module, output, ignored_template_inputs, docker_image,
	docker_index, ignore_template_numbers, verbose):
	"""Nipype to Boutiques exporter.

	See Boutiques specification at https://github.com/boutiques/schema.
	"""
	from nipype.utils.nipype2boutiques import generate_boutiques_descriptor

	# Generates JSON string
	json_string = generate_boutiques_descriptor(
	module, interface, ignored_template_inputs, docker_image, docker_index,
	verbose, ignore_template_numbers)


		inp = {}

		if input_number is not None and input_number != 0: # No need to append a number to the first of a list of compound inputs

	if input_number is not None and input_number != 0: # No need to append a number to the first of a list of compound inputs
	if input_number: # No need to append a number to the first of a list of compound inputs

	path = save_path if save_path is not None else os.path.join(os.getcwd(), interface_name + '.json')
	path = save_path or os.path.join(os.getcwd(), interface_name + '.json')

	if trait_handler.maxlen != six.MAXSIZE:
	if trait_handler.maxlen != sys.maxsize:

REF: Update nipype2boutiques script #2894

REF: Update nipype2boutiques script #2894

Conversation

erinb90 commented Feb 28, 2019

Summary

Acknowledgment

codecov-io commented Feb 28, 2019 • edited Loading

Codecov Report

glatard commented Feb 28, 2019

mgxd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mgxd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mgxd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erinb90 commented Mar 29, 2019

mgxd commented Apr 1, 2019

mgxd commented Apr 9, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Feb 28, 2019 •

edited

Loading