Repository: PMEAL/OpenPNM
Pull request #372 (2015-06-01 16:29:51)

Fix __str__ methods
During my PEP8ification of OpenPNM I broke a few string methods. This PR should that as well as doing an extra check when initializing a `ModelWraper`.
@maghighi @jgostick 

jgostick: This doesn't seem right.  I haven't run it, but the changes to models dict seem a bit drastic, and what about Core, and the printable dict/lists?  
jgostick: Core prints fine, sorry about that...print printable lists don't
bspellio: @jgostick The reason for the change is that `__str__` is supposed to return a string, not call print many times.
jgostick: OK, understood.

On Mon, Jun 1, 2015 at 12:55 PM, Brennan Spellacy <notifications@github.com>
wrote:

> @jgostick <https://github.com/jgostick> The reason for the change is that
> __str__ is supposed to return a string, not call print many times.
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/PMEAL/OpenPNM/pull/372#issuecomment-107636170>.
>

coveralls: 
[![Coverage Status](https://coveralls.io/builds/2699727/badge)](https://coveralls.io/builds/2699727)

Changes Unknown when pulling **6b00f7142777f1e14958f1dde7167f31b96d6fae on fix-string-methods** into ** on develop**.

jgostick: Perhaps all of the __str__ methods should be converted to return a string?
bspellio: They absolutely should be. I'll add that to this PR.
bspellio: @jgostick good to merge this?

Patch:
diff --git a/OpenPNM/Base/__Controller__.py b/OpenPNM/Base/__Controller__.py
index e0c931f7e..51524e73f 100644
--- a/OpenPNM/Base/__Controller__.py
+++ b/OpenPNM/Base/__Controller__.py
@@ -29,41 +29,45 @@ class Controller(dict):
         self.comments = 'Using OpenPNM ' + OpenPNM.__version__
 
     def __str__(self):
+        lines = []
+        horizontal_rule = 60 * '-'
         for net in self.networks():
-            header = '-'*60
-            print(header)
-            print('{a:<15} {b:<20} ({c})'.format(a='Object: ', b='Name', c='Class'))
-            print(header)
-            print('{a:<15} {b:<20} ({c})'.format(a='Network: ',
-                                                 b=net.name,
-                                                 c=net.__class__.__name__))
+            lines.append(horizontal_rule)
+            lines.append('{0:<15} {1:<20} ({2})'.format('Object:',
+                                                        'Name',
+                                                        'Class'))
+            lines.append(horizontal_rule)
+            lines.append('{0:<15} {1:<20} ({2})'.format('Network:',
+                                                        net.name,
+                                                        net.__class__.__name__))
             for geom in net._geometries:
-                print('++ {a:<12} {b:<20} ({c})'.format(a='Geometry: ',
-                                                        b=geom.name,
-                                                        c=geom.__class__.__name__))
+                str = '++ {0:<12} {1:<20} ({2})'
+                lines.append(str.format('Geometry: ',
+                                        geom.name,
+                                        geom.__class__.__name__))
             for phase in net._phases:
                 if len(phase._phases) == 0:
-                    str = '+ {a:<13} {b:<20} ({c})'
-                    print(str.format(a='Pure Phase: ',
-                                     b=phase.name,
-                                     c=phase.__class__.__name__))
+                    str = '+ {0:<13} {1:<20} ({2})'
+                    lines.append(str.format('Pure Phase: ',
+                                            phase.name,
+                                            phase.__class__.__name__))
                 if len(phase._phases) > 1:
-                    str = '+ {a:<13} {b:<20} ({c})'
-                    print(str.format(a='Mixture Phase: ',
-                                     b=phase.name,
-                                     c=phase.__class__.__name__))
+                    str = '+ {0:<13} {1:<20} ({2})'
+                    lines.append(str.format('Mixture Phase: ',
+                                            phase.name,
+                                            phase.__class__.__name__))
                     comps = phase.phases()
                     for compname in comps:
-                        str = '++ {a:<12} {b:<20} ({c})}'
-                        print(str.format(a='Component Phase: ',
-                                         b=phase.name,
-                                         c=phase.__class__.__name__))
+                        str = '++ {0:<12} {1:<20} ({2})}'
+                        lines.append(str.format('Component Phase: ',
+                                                phase.name,
+                                                phase.__class__.__name__))
                 for phys in phase._physics:
-                    str = '++ {a:<12} {b:<20} ({c})'
-                    print(str.format(a='Physics: ',
-                                     b=phys.name,
-                                     c=phys.__class__.__name__))
-        return ''
+                    str = '++ {0:<12} {1:<20} ({2})'
+                    lines.append(str.format('Physics: ',
+                                            phys.name,
+                                            phys.__class__.__name__))
+        return '\n'.join(lines)
 
     def _setloglevel(self, level):
         logger.setLevel(level)
diff --git a/OpenPNM/Base/__Core__.py b/OpenPNM/Base/__Core__.py
index 75effaf00..b7eac0a6a 100644
--- a/OpenPNM/Base/__Core__.py
+++ b/OpenPNM/Base/__Core__.py
@@ -67,18 +67,18 @@ class Core(dict):
         100
 
         """
-        #Enforce correct dict naming
+        # Enforce correct dict naming
         element = key.split('.')[0]
         if (element != 'pore') and (element != 'throat'):
             print('Array name \''+key+'\' does not begin with \'pore\' or \'throat\'')
             return
-        #Convert value to an ndarray
+        # Convert value to an ndarray
         value = sp.array(value,ndmin=1)
         #Skip checks for 'coords', 'conns'
         if (key == 'pore.coords') or (key == 'throat.conns'):
             super(Core, self).__setitem__(key,value)
             return
-        #Skip checks for protected props, and prevent changes if defined
+        # Skip checks for protected props, and prevent changes if defined
         if key.split('.')[1] in ['all']:
             if key in self.keys():
                 if sp.shape(self[key]) == (0,):
@@ -91,7 +91,7 @@ class Core(dict):
                 logger.debug(key+' is being defined.')
                 super(Core, self).__setitem__(key,value)
             return
-        #Write value to dictionary
+        # Write value to dictionary
         if sp.shape(value)[0] == 1:  # If value is scalar
             logger.debug('Broadcasting scalar value into vector: '+key)
             value = sp.ones((self._count(element),),dtype=value.dtype)*value
@@ -396,7 +396,6 @@ class Core(dict):
             if element == '': temp = constants
             else: temp = [item for item in constants if item.split('.')[0]==element]
         a = Tools.PrintableList(temp)
-        # a.sort()
         return a
 
 
@@ -1471,36 +1470,42 @@ class Core(dict):
         return health
 
     def __str__(self):
-        horizonal_rule = '-'*60
-        lines  = [horizonal_rule]
-        lines += [self.__module__.replace('__','')+': \t'+self.name]
-        lines += [horizonal_rule]
-        lines += ["{a:<5s} {b:<35s} {c:<10s}".format(a='#', b='Properties', c='Valid Values')]
-        lines += [horizonal_rule]
-        count = 0
+        horizonal_rule = '-' * 60
+        lines = [horizonal_rule]
+        lines.append(self.__module__.replace('__', '') + ': \t' + self.name)
+        lines.append(horizonal_rule)
+        lines.append("{0:<5s} {1:<35s} {2:<10s}".format('#',
+                                                        'Properties',
+                                                        'Valid Values'))
+        lines.append(horizonal_rule)
         props = self.props()
         props.sort()
-        for item in props:
+        for i, item in enumerate(props):
             if self[item].dtype != object:
-                count = count + 1
-                prop=item
-                if len(prop)>35:
-                    prop = prop[0:32]+'...'
+                prop = item
+                if len(prop) > 35:
+                    prop = prop[0:32] + '...'
                 required = self._count(item.split('.')[0])
                 a = sp.isnan(self[item])
-                defined = sp.shape(self[item])[0] - a.sum(axis=0,keepdims=(a.ndim-1)==0)[0]
-                lines += ["{a:<5d} {b:<35s} {c:>5d} / {d:<5d}".format(a=count, b=prop, c=defined, d=required)]
-        lines += [horizonal_rule]
-        lines += ["{a:<5s} {b:<35s} {c:<10s}".format(a='#', b='Labels', c='Assigned Locations')]
-        lines += [horizonal_rule]
-        count = 0
+                defined = sp.shape(self[item])[0] - a.sum(axis=0,
+                                                          keepdims=(a.ndim-1)==0)[0]
+                lines.append("{0:<5d} {1:<35s} {2:>5d} / {3:<5d}".format(i + 1,
+                                                                         prop,
+                                                                         defined,
+                                                                         required))
+        lines.append(horizonal_rule)
+        lines.append("{0:<5s} {1:<35s} {2:<10s}".format('#',
+                                                        'Labels',
+                                                        'Assigned Locations'))
+        lines.append(horizonal_rule)
         labels = self.labels()
         labels.sort()
-        for item in labels:
-            count = count + 1
-            prop=item
+        for i, item in enumerate(labels):
+            prop = item
             if len(prop)>35:
-                prop = prop[0:32]+'...'
-            lines += ["{a:<5d} {b:<35s} {c:<10d}".format(a=count, b=prop, c=sp.sum(self[item]))]
-        lines += [horizonal_rule]
+                prop = prop[0:32] + '...'
+            lines.append("{0:<5d} {1:<35s} {2:<10d}".format(i + 1,
+                                                            prop,
+                                                            sp.sum(self[item])))
+        lines.append(horizonal_rule)
         return '\n'.join(lines)
diff --git a/OpenPNM/Base/__ModelsDict__.py b/OpenPNM/Base/__ModelsDict__.py
index be1a02a83..a8ca8bf62 100644
--- a/OpenPNM/Base/__ModelsDict__.py
+++ b/OpenPNM/Base/__ModelsDict__.py
@@ -26,12 +26,14 @@ class ModelWrapper(dict):
         return self['model'](**self)
 
     def __str__(self):
-        header = '-' * 60
-        print(header)
-        print(self['model'].__module__ + '.' + self['model'].__name__)
-        print(header)
-        print('{a:<20s} {b}'.format(a='Argument Name', b='Value / (Default)'))
-        print(header)
+        if self['model'] is None:
+            return 'No model specified.'
+        horizontal_rule = '-' * 60
+        lines = [horizontal_rule]
+        lines.append(self['model'].__module__ + '.' + self['model'].__name__)
+        lines.append(horizontal_rule)
+        lines.append('{0:<20s} {1}'.format('Argument Name', 'Value / (Default)'))
+        lines.append(horizontal_rule)
         # Scan default argument names and values of model
         defs = {}
         if self['model'].__defaults__ is not None:
@@ -45,11 +47,11 @@ class ModelWrapper(dict):
             if item not in self.COMPONENTS:
                 if item not in defs.keys():
                     defs[item] = '---'
-                print('{a:<20s} {b} / ({c})'.format(a=item,
-                                                    b=self[item],
-                                                    c=defs[item]))
-        print(header)
-        return ' '
+                lines.append('{0:<20s} {1} / ({2})'.format(item,
+                                                          self[item],
+                                                          defs[item]))
+        lines.append(horizontal_rule)
+        return '\n'.join(lines)
 
     def regenerate(self):
         r"""
@@ -129,19 +131,17 @@ class ModelsDict(OrderedDict):
         super().__setitem__(propname, temp)
 
     def __str__(self):
-        horizontal_rule = ('-' * 60) + '\n'
-        string = ''
-        string += horizontal_rule
-        string += '{n:<5s} {a:<30s} {b}\n'.format(n='#',
-                                                  a='Property Name',
-                                                  b='Regeneration Mode')
-        string += horizontal_rule
-        for item, i in self.keys():
-            string += '{n:<5d} {a:<30s} {b:<20s}'.format(n=(i + 1),
-                                                         a=item,
-                                                         b=self[item]['regen_mode'])
-        string += horizontal_rule
-        return string
+        horizontal_rule = '-' * 60
+        lines = [horizontal_rule]
+        lines.append('{0:<5s} {1:<30s} {2}'.format('#',
+                                                   'Property Name',
+                                                   'Regeneration Mode'))
+        lines.append(horizontal_rule)
+        for i, item in enumerate(self.keys()):
+            str = '{0:<5d} {1:<30s} {2:<20s}'
+            lines.append(str.format(i + 1, item, self[item]['regen_mode']))
+        lines.append(horizontal_rule)
+        return '\n'.join(lines)
 
     def keys(self):
         return list(super().keys())
@@ -264,7 +264,6 @@ class ModelsDict(OrderedDict):
         regen_mode           normal / (---)
         seed                 None / (None)
         ------------------------------------------------------------
-        <BLANKLINE>
         """
         master = self._find_master()
         if master is None:
diff --git a/OpenPNM/Base/__Tools__.py b/OpenPNM/Base/__Tools__.py
index 508d85ab1..a21bac0f1 100644
--- a/OpenPNM/Base/__Tools__.py
+++ b/OpenPNM/Base/__Tools__.py
@@ -9,15 +9,13 @@ from collections import OrderedDict as _odict
 
 class PrintableList(list):
     def __str__(self):
-        horizonal_rule = ('-' * 60) + '\n'
-        string = ''
-        string += '\n'
-        string += horizonal_rule
+        horizontal_rule = '-' * 60
+        lines = [horizontal_rule]
         self.sort()
         for i, item in enumerate(self):
-            string += '{0}\t: {1}'.format(i + 1, item)
-        string += horizonal_rule
-        return string
+            lines.append('{0}\t: {1}'.format(i + 1, item))
+        lines.append(horizontal_rule)
+        return '\n'.join(lines)
 
 
 class PrintableDict(_odict):
@@ -26,14 +24,14 @@ class PrintableDict(_odict):
         return text
 
     def __str__(self):
-        header = '-' * 60
-        print(header)
-        print('{a:<25s} {b:<25s}'.format(a='key', b='value'))
-        print(header)
+        horizontal_rule = '-' * 60
+        lines = [horizontal_rule]
+        lines.append('{0:<25s} {1}'.format('key', 'value'))
+        lines.append(horizontal_rule)
         for item in self.keys():
-            print('{a:<25s} {b}'.format(a=item, b=self[item]))
-        print(header)
-        return ''
+            lines.append('{0:<25s} {1}'.format(item, self[item]))
+        lines.append(horizontal_rule)
+        return '\n'.join(lines)
 
 
 class AttributVeiew(object):
diff --git a/docs/_extensions/docscrape.py b/docs/_extensions/docscrape.py
index f374b3ddc..9538f9e5b 100644
--- a/docs/_extensions/docscrape.py
+++ b/docs/_extensions/docscrape.py
@@ -183,7 +183,7 @@ class NumpyDocString(object):
 
         return params
 
-    
+
     _name_rgx = re.compile(r"^\s*(:(?P<role>\w+):`(?P<name>[a-zA-Z0-9_.-]+)`|"
                            r" (?P<name2>[a-zA-Z0-9_.-]+))\s*", re.X)
     def _parse_see_also(self, content):
@@ -216,7 +216,7 @@ class NumpyDocString(object):
 
         current_func = None
         rest = []
-        
+
         for line in content:
             if not line.strip(): continue
 
@@ -258,7 +258,7 @@ class NumpyDocString(object):
             if len(line) > 2:
                 out[line[1]] = strip_each_in(line[2].split(','))
         return out
-    
+
     def _parse_summary(self):
         """Grab signature (if given) and summary"""
         if self._is_at_section():
@@ -275,7 +275,7 @@ class NumpyDocString(object):
 
         if not self._is_at_section():
             self['Extended Summary'] = self._read_to_next_section()
-    
+
     def _parse(self):
         self._doc.reset()
         self._parse_summary()
@@ -440,7 +440,7 @@ class FunctionDoc(NumpyDocString):
         else:
             func = self._f
         return func, func_name
-            
+
     def __str__(self):
         out = ''
 
@@ -493,5 +493,3 @@ class ClassDoc(NumpyDocString):
         #    out += '.. index::\n   single: %s; %s\n\n' % (self._name, m)
 
         return out
-
-


Test patch:
diff --git a/bin/test b/bin/test
index f616c304a..9466ccdf4 100755
--- a/bin/test
+++ b/bin/test
@@ -3,7 +3,7 @@
 import sys
 import subprocess
 
-command = ['py.test']
+command = ['py.test', '-v']
 
 if 'pep8' in sys.argv:
     command.append('--pep8')
diff --git a/test/unit/Base/ControllerTest.py b/test/unit/Base/ControllerTest.py
index 1c5609b98..cf885b54f 100644
--- a/test/unit/Base/ControllerTest.py
+++ b/test/unit/Base/ControllerTest.py
@@ -1,13 +1,22 @@
 from OpenPNM.Base import Controller
+from OpenPNM.Network import GenericNetwork
 
 
 class ControllerTest:
 
     def setup_class(self):
         self.controller = Controller()
+        self.pn = GenericNetwork(name='test_net')
 
     def test_str(self):
-        pass
+        actual_string = self.controller.__str__()
+        expected_string = \
+            '------------------------------------------------------------\n' + \
+            'Object:         Name                 (Class)\n' + \
+            '------------------------------------------------------------\n' + \
+            'Network:        test_net             (GenericNetwork)'
+
+        assert actual_string == expected_string
 
     def test_get_log_level(self):
         self.controller.loglevel = 50
@@ -15,3 +24,4 @@ class ControllerTest:
 
     def teardown_class(self):
         del(self.controller)
+        del(self.pn)
diff --git a/test/unit/Base/ModelsDictTest.py b/test/unit/Base/ModelsDictTest.py
index d5d04d479..55b092491 100644
--- a/test/unit/Base/ModelsDictTest.py
+++ b/test/unit/Base/ModelsDictTest.py
@@ -1,11 +1,14 @@
 from OpenPNM.Base import ModelsDict
+from OpenPNM.Base.__ModelsDict__ import ModelWrapper
 
 
 class ModelsDictTest:
 
-    class ModelsWrapperTest:
-        def setup_class(self):
-            pass
+    class ModelWrapperTest:
+        def test_str_with_none_model(self):
+            actual_string = ModelWrapper(model=None).__str__()
+            expected_string = 'No model specified.'
+            assert expected_string == actual_string
 
         def test_find_master_with_more_than_one_master(self):
             pass
@@ -20,7 +23,7 @@ class ModelsDictTest:
                 '------------------------------------------------------------\n' + \
                 '#     Property Name                  Regeneration Mode\n' + \
                 '------------------------------------------------------------\n' + \
-                '------------------------------------------------------------\n'
+                '------------------------------------------------------------'
             assert expected_string == actual_string
 
         def test_str_with_models(self):
diff --git a/test/unit/Base/ToolsTest.py b/test/unit/Base/ToolsTest.py
index d48c45bb5..0fcb5c4ae 100644
--- a/test/unit/Base/ToolsTest.py
+++ b/test/unit/Base/ToolsTest.py
@@ -8,9 +8,26 @@ class ToolsTest:
 
         def test_str(self):
             actual_string = self.list.__str__()
-            expected_string = '\n' + \
+            expected_string = \
                 '------------------------------------------------------------\n' + \
-                '1\t: first' + \
-                '2\t: second' + \
-                '------------------------------------------------------------\n'
+                '1\t: first\n' + \
+                '2\t: second\n' + \
+                '------------------------------------------------------------'
+            assert actual_string == expected_string
+
+    class PrintableDictTest:
+        def setup_class(self):
+            self.dict = Tools.PrintableDict()
+            self.dict['first_key'] = 'first_value'
+            self.dict['second_key'] = 'second_value'
+
+        def test_str(self):
+            actual_string = self.dict.__str__()
+            expected_string = \
+                '------------------------------------------------------------\n' + \
+                'key                       value\n' + \
+                '------------------------------------------------------------\n' + \
+                'first_key                 first_value\n' + \
+                'second_key                second_value\n' + \
+                '------------------------------------------------------------'
             assert actual_string == expected_string