Repository: nipy/nipype

Issue:
incorrect detailed graphs being generated
### Summary

the detailed graph is not listing names of nodes appropriately resulting in incorrect graphs.

scroll down to see the multiple arrows containing nodes in the figures of the following notebook.
https://miykael.github.io/nipype_tutorial/notebooks/introduction_quickstart.html

an example here:

![image](https://user-images.githubusercontent.com/184063/43534422-50e73862-9585-11e8-83d9-697675db63e8.png)

i suspect this has something to do with how `__repr__` and `__str__` are being used. 

@oesteban, @effigies  - any quick ideas before i look into the code?

### Actual behavior

### Expected behavior

should have printed the old style with multiple nodes. 

see cell 14 in this notebook:
https://github.com/ReproNim/reproducible-imaging/blob/master/notebooks/introductory_dataflows.ipynb

### How to replicate the behavior

run the current quickstart notebook in @miykael binder repo.



Discussion:

Patch:
diff --git a/nipype/pipeline/engine/base.py b/nipype/pipeline/engine/base.py
index 9d0bc3c69..7f7afd392 100644
--- a/nipype/pipeline/engine/base.py
+++ b/nipype/pipeline/engine/base.py
@@ -36,11 +36,11 @@ class EngineBase(object):
 
         """
         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 @@ class EngineBase(object):
     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 @@ class EngineBase(object):
     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
index 5ac9e72fa..af93fd140 100644
--- a/nipype/pipeline/engine/nodes.py
+++ b/nipype/pipeline/engine/nodes.py
@@ -159,7 +159,6 @@ class Node(EngineBase):
         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 @@ class Node(EngineBase):
         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/utils.py b/nipype/pipeline/engine/utils.py
index 4ec36afe6..0bb5351ad 100644
--- a/nipype/pipeline/engine/utils.py
+++ b/nipype/pipeline/engine/utils.py
@@ -507,7 +507,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']:
@@ -519,8 +519,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'] + [


Test patch:
diff --git a/nipype/pipeline/engine/tests/test_engine.py b/nipype/pipeline/engine/tests/test_engine.py
index 151849241..44afbb2e2 100644
--- 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