Merge pull request #2669 from djarecka/fix/wf_graph

effigies · web-flow · commit d2abc85cd98e · 2018-08-07T08:33:24.000-04:00
FIX: Node __repr__ and detailed graph expansion
diff --git a/nipype/pipeline/engine/base.py b/nipype/pipeline/engine/base.py
@@ -36,11 +36,11 @@ def __init__(self, name=None, base_dir=None):
 
         """
         self._hierarchy = None
-        self._name = None
+        self.name = name
+        self._id = self.name # for compatibility with node expansion using iterables
 
         self.base_dir = base_dir
         self.config = deepcopy(config._sections)
-        self.name = name
 
     @property
     def name(self):
@@ -66,6 +66,14 @@ def inputs(self):
     def outputs(self):
         raise NotImplementedError
 
+    @property
+    def itername(self):
+        """Name for expanded iterable"""
+        itername = self._id
+        if self._hierarchy:
+            itername = '%s.%s' % (self._hierarchy, self._id)
+        return itername
+
     def clone(self, name):
         """Clone an EngineBase object
 
@@ -95,6 +103,9 @@ def _check_inputs(self, parameter):
     def __str__(self):
         return self.fullname
 
+    def __repr__(self):
+        return self.itername
+
     def save(self, filename=None):
         if filename is None:
             filename = 'temp.pklz'
diff --git a/nipype/pipeline/engine/nodes.py b/nipype/pipeline/engine/nodes.py
@@ -159,7 +159,6 @@ def __init__(self,
         self._got_inputs = False
         self._originputs = None
         self._output_dir = None
-        self._id = self.name  # for compatibility with node expansion using iterables
 
         self.iterables = iterables
         self.synchronize = synchronize
@@ -249,14 +248,6 @@ def n_procs(self, value):
         if hasattr(self._interface.inputs, 'num_threads'):
             self._interface.inputs.num_threads = self._n_procs
 
-    @property
-    def itername(self):
-        """Name for expanded iterable"""
-        itername = self._id
-        if self._hierarchy:
-            itername = '%s.%s' % (self._hierarchy, self._id)
-        return itername
-
     def output_dir(self):
         """Return the location of the output directory for the node"""
         # Output dir is cached
diff --git a/nipype/pipeline/engine/tests/test_engine.py b/nipype/pipeline/engine/tests/test_engine.py
@@ -441,6 +441,7 @@ def test_write_graph_runs(tmpdir):
 
             assert os.path.exists('graph.dot') or os.path.exists(
                 'graph_detailed.dot')
+       
             try:
                 os.remove('graph.dot')
             except OSError:
@@ -484,6 +485,164 @@ def test_deep_nested_write_graph_runs(tmpdir):
                 pass
 
 
+# examples of dot files used in the following test
+dotfile_orig = ['strict digraph  {\n',
+                '"mod1 (engine)";\n',
+                '"mod2 (engine)";\n',
+                '"mod1 (engine)" -> "mod2 (engine)";\n',
+                '}\n']
+
+dotfile_detailed_orig = ['digraph structs {\n',
+                         'node [shape=record];\n',
+                         'pipemod1 [label="{IN}|{ mod1 | engine |  }|{OUT|<outoutput1> output1}"];\n',
+                         'pipemod2 [label="{IN|<ininput1> input1}|{ mod2 | engine |  }|{OUT}"];\n',
+                         'pipemod1:outoutput1:e -> pipemod2:ininput1:w;\n',
+                         '}']
+
+
+dotfile_hierarchical = ['digraph pipe{\n',
+                        '  label="pipe";\n',
+                        '  pipe_mod1[label="mod1 (engine)"];\n',
+                        '  pipe_mod2[label="mod2 (engine)"];\n',
+                        '  pipe_mod1 -> pipe_mod2;\n',
+                        '}']
+
+dotfile_colored = ['digraph pipe{\n',
+                   '  label="pipe";\n',
+                   '  pipe_mod1[label="mod1 (engine)", style=filled, fillcolor="#FFFFC8"];\n',
+                   '  pipe_mod2[label="mod2 (engine)", style=filled, fillcolor="#FFFFC8"];\n',
+                   '  pipe_mod1 -> pipe_mod2;\n',
+                   '}']
+
+dotfiles = {
+    "orig": dotfile_orig,
+    "flat": dotfile_orig,
+    "exec": dotfile_orig,
+    "hierarchical": dotfile_hierarchical,
+    "colored": dotfile_colored
+    }
+
+@pytest.mark.parametrize("simple", [True, False])
+@pytest.mark.parametrize("graph_type", ['orig', 'flat', 'exec', 'hierarchical', 'colored'])
+def test_write_graph_dotfile(tmpdir, graph_type, simple):
+    """ checking dot files for a workflow without iterables"""
+    tmpdir.chdir()
+
+    pipe = pe.Workflow(name='pipe')
+    mod1 = pe.Node(interface=EngineTestInterface(), name='mod1')
+    mod2 = pe.Node(interface=EngineTestInterface(), name='mod2')
+    pipe.connect([(mod1, mod2, [('output1', 'input1')])])
+    pipe.write_graph(
+        graph2use=graph_type, simple_form=simple, format='dot')
+
+    with open("graph.dot") as f:
+        graph_str = f.read()
+
+    if simple:
+        for line in dotfiles[graph_type]:
+            assert line in graph_str
+    else:
+        # if simple=False graph.dot uses longer names
+        for line in dotfiles[graph_type]:
+            if graph_type in ["hierarchical", "colored"]:
+                assert line.replace("mod1 (engine)", "mod1.EngineTestInterface.engine").replace(
+                    "mod2 (engine)", "mod2.EngineTestInterface.engine") in graph_str
+            else:
+                assert line.replace(
+                    "mod1 (engine)", "pipe.mod1.EngineTestInterface.engine").replace(
+                    "mod2 (engine)", "pipe.mod2.EngineTestInterface.engine") in graph_str
+
+    # graph_detailed is the same for orig, flat, exec (if no iterables)
+    # graph_detailed is not created for hierachical or colored
+    if graph_type not in ["hierarchical", "colored"]:
+        with open("graph_detailed.dot") as f:
+            graph_str = f.read()
+        for line in dotfile_detailed_orig:
+            assert line in graph_str
+
+
+# examples of dot files used in the following test
+dotfile_detailed_iter_exec = [
+    'digraph structs {\n',
+    'node [shape=record];\n',
+    'pipemod1aIa1 [label="{IN}|{ a1 | engine | mod1.aI }|{OUT|<outoutput1> output1}"];\n',
+    'pipemod2a1 [label="{IN|<ininput1> input1}|{ a1 | engine | mod2 }|{OUT}"];\n',
+    'pipemod1aIa0 [label="{IN}|{ a0 | engine | mod1.aI }|{OUT|<outoutput1> output1}"];\n',
+    'pipemod2a0 [label="{IN|<ininput1> input1}|{ a0 | engine | mod2 }|{OUT}"];\n',
+    'pipemod1aIa0:outoutput1:e -> pipemod2a0:ininput1:w;\n',
+    'pipemod1aIa1:outoutput1:e -> pipemod2a1:ininput1:w;\n',
+    '}']
+
+dotfile_iter_hierarchical = [
+    'digraph pipe{\n',
+    '  label="pipe";\n',
+    '  pipe_mod1[label="mod1 (engine)", shape=box3d,style=filled, color=black, colorscheme=greys7 fillcolor=2];\n',
+    '  pipe_mod2[label="mod2 (engine)"];\n',
+    '  pipe_mod1 -> pipe_mod2;\n',
+    '}']
+
+dotfile_iter_colored = [
+    'digraph pipe{\n',
+    '  label="pipe";\n',
+    '  pipe_mod1[label="mod1 (engine)", shape=box3d,style=filled, color=black, colorscheme=greys7 fillcolor=2];\n',
+    '  pipe_mod2[label="mod2 (engine)", style=filled, fillcolor="#FFFFC8"];\n',
+    '  pipe_mod1 -> pipe_mod2;\n',
+    '}']
+
+dotfiles_iter = {
+    "orig": dotfile_orig,
+    "flat": dotfile_orig,
+    "exec": dotfile_orig,
+    "hierarchical": dotfile_iter_hierarchical,
+    "colored": dotfile_iter_colored
+    }
+
+dotfiles_detailed_iter = {
+    "orig": dotfile_detailed_orig,
+    "flat": dotfile_detailed_orig,
+    "exec": dotfile_detailed_iter_exec
+    }
+
+@pytest.mark.parametrize("simple", [True, False])
+@pytest.mark.parametrize("graph_type", ['orig', 'flat', 'exec', 'hierarchical', 'colored'])
+def test_write_graph_dotfile_iterables(tmpdir, graph_type, simple):
+    """ checking dot files for a workflow with iterables"""
+    tmpdir.chdir()
+
+    pipe = pe.Workflow(name='pipe')
+    mod1 = pe.Node(interface=EngineTestInterface(), name='mod1')
+    mod1.iterables = ('input1', [1, 2])
+    mod2 = pe.Node(interface=EngineTestInterface(), name='mod2')
+    pipe.connect([(mod1, mod2, [('output1', 'input1')])])
+    pipe.write_graph(
+        graph2use=graph_type, simple_form=simple, format='dot')
+
+    with open("graph.dot") as f:
+        graph_str = f.read()
+
+    if simple:
+        for line in dotfiles_iter[graph_type]:
+            assert line in graph_str
+    else:
+        # if simple=False graph.dot uses longer names
+        for line in dotfiles_iter[graph_type]:
+            if graph_type in ["hierarchical", "colored"]:
+                assert line.replace("mod1 (engine)", "mod1.EngineTestInterface.engine").replace(
+                    "mod2 (engine)", "mod2.EngineTestInterface.engine") in graph_str
+            else:
+                assert line.replace(
+                    "mod1 (engine)", "pipe.mod1.EngineTestInterface.engine").replace(
+                    "mod2 (engine)", "pipe.mod2.EngineTestInterface.engine") in graph_str
+
+    # graph_detailed is not created for hierachical or colored
+    if graph_type not in ["hierarchical", "colored"]:
+        with open("graph_detailed.dot") as f:
+            graph_str = f.read()
+        for line in dotfiles_detailed_iter[graph_type]:
+            assert line in graph_str
+
+
+
 def test_io_subclass():
     """Ensure any io subclass allows dynamic traits"""
     from nipype.interfaces.io import IOBase
diff --git a/nipype/pipeline/engine/utils.py b/nipype/pipeline/engine/utils.py
@@ -570,7 +570,7 @@ def _write_detailed_dot(graph, dotfilename):
     # write nodes
     edges = []
     for n in nx.topological_sort(graph):
-        nodename = str(n)
+        nodename = n.itername
         inports = []
         for u, v, d in graph.in_edges(nbunch=n, data=True):
             for cd in d['connect']:
@@ -582,8 +582,8 @@ def _write_detailed_dot(graph, dotfilename):
                 ipstrip = 'in%s' % _replacefunk(inport)
                 opstrip = 'out%s' % _replacefunk(outport)
                 edges.append(
-                    '%s:%s:e -> %s:%s:w;' % (str(u).replace('.', ''), opstrip,
-                                             str(v).replace('.', ''), ipstrip))
+                    '%s:%s:e -> %s:%s:w;' % (u.itername.replace('.', ''), opstrip,
+                                             v.itername.replace('.', ''), ipstrip))
                 if inport not in inports:
                     inports.append(inport)
         inputstr = ['{IN'] + [
