Repository: PMEAL/OpenPNM
Pull request #1068 (2018-08-21 02:58:47)

Fix 1066 prop naming
Adding a check to setitem in Base so that trying to add 'pore.foo' will raise an exception when 'pore.foo.bar' is already present, and vice-versa.  (This time PRing to dev instad of master)


Patch:
diff --git a/example_script_two_subdomains.py b/example_script_two_subdomains.py
new file mode 100644
index 000000000..d2957f313
--- /dev/null
+++ b/example_script_two_subdomains.py
@@ -0,0 +1,51 @@
+import pytest
+import openpnm as op
+ws = op.Workspace()
+proj = ws.new_project()
+
+pn = op.network.Cubic(shape=[10, 10, 10], spacing=1e-4, project=proj)
+Ps = pn['pore.coords'][:, 0] < pn['pore.coords'][:, 0].mean()
+Ts = pn.find_neighbor_throats(pores=Ps, mode='xnor')
+geo1 = op.geometry.StickAndBall(network=pn, pores=Ps, throats=Ts)
+
+Ps = pn['pore.coords'][:, 0] >= pn['pore.coords'][:, 0].mean()
+Ts = pn.find_neighbor_throats(pores=Ps, mode='or')
+geo2 = op.geometry.StickAndBall(network=pn, pores=Ps, throats=Ts)
+
+pn['pore.foo'] = 1
+# Can't create a subdict below foo
+with pytest.raises(Exception):
+    pn['pore.foo.bar'] = 1
+# Can create a subdict directly
+pn['pore.baz.bar'] = 2
+# Can't create a new item already used as subdict
+with pytest.raises(Exception):
+    pn['pore.baz'] = 2
+
+# Also works on subdomains
+geo1['pore.blah'] = 1
+with pytest.raises(Exception):
+    geo1['pore.blah.boo'] = 1
+geo1['pore.bee.bop'] = 1
+with pytest.raises(Exception):
+    geo1['pore.bee'] = 1
+
+# Now start looking across objects
+with pytest.raises(Exception):
+    geo1['pore.foo'] = 1  # Already exists on pn
+with pytest.raises(Exception):
+    geo1['pore.foo.bar'] = 1  # pore.foo already exists on pn
+with pytest.raises(Exception):
+    geo1['pore.baz'] = 1  # pore.baz.bar already exists on pn
+
+# Now start looking across objects
+geo2['pore.blah'] = 1
+geo2['pore.bee.bop'] = 1
+with pytest.raises(Exception):
+    geo1['pore.bee'] = 1
+
+with pytest.raises(Exception):
+    pn['pore.bee'] = 1
+
+with pytest.raises(Exception):
+    pn['pore.bee.bop'] = 1
diff --git a/openpnm/core/Base.py b/openpnm/core/Base.py
index f54d90632..b25d72b06 100644
--- a/openpnm/core/Base.py
+++ b/openpnm/core/Base.py
@@ -172,16 +172,42 @@ class Base(dict):
         'throat.***'.  Also, any scalars are cast into full length vectors.
 
         """
-        # If value is a dictionary, then break it up into constituent arrays
+        # Check 1: If value is a dictionary, break it into constituent arrays
         if hasattr(value, 'keys'):
             for item in value.keys():
                 prop = item.replace('pore.', '').replace('throat.', '')
                 self.__setitem__(key+'.'+prop, value[item])
             return
 
+        # Check 2: If adding a new key, make sure it has no conflicts
+        if self.project:
+            proj = self.project
+            boss = proj.find_full_domain(self)
+            keys = boss.keys(mode='all', deep=True)
+        else:
+            boss = None
+            keys = self.keys()
+        # Prevent 'pore.foo.bar' when 'pore.foo' present
+        long_keys = [i for i in keys if i.count('.') > 1]
+        key_root = '.'.join(key.split('.')[:2])
+        if (key.count('.') > 1) and (key_root in keys):
+            raise Exception('Cannot create ' + key + ' when ' +
+                            key_root + ' is already defined')
+        # Prevent 'pore.foo' when 'pore.foo.bar' is present
+        if (key.count('.') == 1) and any([i.startswith(key) for i in long_keys]):
+            hit = [i for i in keys if i.startswith(key)][0]
+            raise Exception('Cannot create ' + key + ' when ' +
+                            hit + ' is already defined')
+        # Prevent writing pore.foo on boss when present on subdomain
+        if boss:
+            if boss is self:
+                if (key in keys) and (key not in self.keys()):
+                    raise Exception('Cannot create ' + key + ' when it is' +
+                                    ' already defined on a subdomain')
+
         value = sp.array(value, ndmin=1)  # Convert value to an ndarray
 
-        # Enforce correct dict naming
+        # Check 3: Enforce correct dict naming
         element = key.split('.')[0]
         element = self._parse_element(element, single=True)
 
@@ -324,7 +350,7 @@ class Base(dict):
                 if item.split('.')[1] not in self.project.names:
                     del self[item]
 
-    def keys(self, element=None, mode=None):
+    def keys(self, element=None, mode=None, deep=False):
         r"""
         This subclass works exactly like ``keys`` when no arguments are passed,
         but optionally accepts an ``element`` and/or a ``mode``, which filters
@@ -340,12 +366,9 @@ class Base(dict):
             keys to only 'pore' or 'throat' keys.  If neither is given, then
             both are assumed.
 
-        mode : string (optional, default is 'skip')
+        mode : string (optional)
             Controls which keys are returned.  Options are:
 
-            **``None``** : This mode (default) bypasses this subclassed method
-            and just returns the normal KeysView object.
-
             **'labels'** : Limits the returned list of keys to only 'labels'
             (boolean arrays)
 
@@ -355,6 +378,13 @@ class Base(dict):
             **'all'** : Returns both 'labels' and 'props'.  This is equivalent
             to sending a list of both 'labels' and 'props'.
 
+            If no mode is specified then the normal KeysView object is
+            returned.
+
+        deep : Boolean
+            If set to ``True`` then the keys on all associated subdomain
+            objects are returned as well.
+
         See Also
         --------
         props
@@ -392,6 +422,15 @@ class Base(dict):
             temp.extend([i for i in keys if self.get(i).dtype == bool])
         if element:
             temp = [i for i in temp if i.split('.')[0] in element]
+
+        if deep:
+            if self._isa('phase'):
+                for item in self.project.find_physics(phase=self):
+                    temp += item.keys(element=element, mode=mode, deep=False)
+            if self._isa('network'):
+                for item in self.project.geometries().values():
+                    temp += item.keys(element=element, mode=mode, deep=False)
+
         return temp
 
     def get(self, keys, default=None):
@@ -475,9 +514,9 @@ class Base(dict):
             **'constants'** : returns data values that were *not* generated by
             a model, but manaully created.
 
-        deep : boolean
-            If ``True`` this will also return the data on any associated
-            subdomain objects
+        deep : Boolean
+            If set to ``True`` then the props on all associated subdomain
+            objects are returned as well.
 
         Returns
         -------
diff --git a/openpnm/core/Subdomain.py b/openpnm/core/Subdomain.py
index 67c34b11a..2358c62c8 100644
--- a/openpnm/core/Subdomain.py
+++ b/openpnm/core/Subdomain.py
@@ -52,11 +52,17 @@ class Subdomain(Base):
         return vals
 
     def __setitem__(self, key, value):
-        if self.project:
-            # Find boss object (either phase or network)
-            boss = self.project.find_full_domain(self)
-            if key in set(boss.keys()).difference({'pore.all', 'throat.all'}):
-                raise Exception(key + ' already exists on ' + boss.name)
+        # If value is a dict, skip all this.  The super class will parse
+        # the dict individually, at which point the below is called.
+        if self.project and not hasattr(value, 'keys'):
+            proj = self.project
+            boss = proj.find_full_domain(self)
+            keys = boss.keys(mode='all', deep=True)
+            # Prevent 'pore.foo' on subdomain when already present on boss
+            if key in set(boss.keys()).difference(set(self.keys())):
+                hit = [i for i in keys if i.startswith(key)][0]
+                raise Exception('Cannot create ' + key + ' when ' +
+                                hit + ' is already defined')
         super().__setitem__(key, value)
 
     def add_locations(self, pores=[], throats=[]):
diff --git a/openpnm/network/Cubic.py b/openpnm/network/Cubic.py
index 2a9828714..a9c5f5bea 100644
--- a/openpnm/network/Cubic.py
+++ b/openpnm/network/Cubic.py
@@ -141,11 +141,11 @@ class Cubic(GenericNetwork):
             heads.extend(H.flat)
         pairs = np.vstack([tails, heads]).T
 
-        self['pore.coords'] = points
-        self['throat.conns'] = pairs
-
         super().__init__(Np=points.shape[0], Nt=pairs.shape[0], name=name,
                          project=project)
+
+        self['pore.coords'] = points
+        self['throat.conns'] = pairs
         # Label faces
         x, y, z = self['pore.coords'].T
         labels = ['front', 'back', 'left', 'right', 'bottom', 'top']
diff --git a/openpnm/network/GenericNetwork.py b/openpnm/network/GenericNetwork.py
index 687242b3e..ed26bd707 100644
--- a/openpnm/network/GenericNetwork.py
+++ b/openpnm/network/GenericNetwork.py
@@ -138,11 +138,6 @@ class GenericNetwork(Base, ModelsMixin):
                     logger.warning('Converting throat.conns to be upper ' +
                                    'triangular')
                     value = sp.sort(value, axis=1)
-        if self.project:
-            for item in self.project.geometries().values():
-                exclude = {'pore.all', 'throat.all'}
-                if key in set(item.keys()).difference(exclude):
-                    raise Exception(key+' already exists on '+item.name)
         super().__setitem__(key, value)
 
     def __getitem__(self, key):
diff --git a/openpnm/phases/GenericPhase.py b/openpnm/phases/GenericPhase.py
index 628499802..c54a15e25 100644
--- a/openpnm/phases/GenericPhase.py
+++ b/openpnm/phases/GenericPhase.py
@@ -78,14 +78,6 @@ class GenericPhase(Base, ModelsMixin):
         self['pore.temperature'] = 298.0
         self['pore.pressure'] = 101325.0
 
-    def __setitem__(self, key, value):
-        if self.project:
-            for item in self.project.find_physics(phase=self):
-                exclude = {'pore.all', 'throat.all'}
-                if key in set(item.keys()).difference(exclude):
-                    raise Exception(key+' already exists on '+item.name)
-        super().__setitem__(key, value)
-
     def __getitem__(self, key):
         element, prop = key.split('.', 1)
         # Deal with special keys first


Test patch:
diff --git a/tests/unit/core/BaseTest.py b/tests/unit/core/BaseTest.py
index 6546acbd2..46a0bca23 100644
--- a/tests/unit/core/BaseTest.py
+++ b/tests/unit/core/BaseTest.py
@@ -600,6 +600,14 @@ class BaseTest:
         # self.geo['pore.all'][0] = False
         # assert sp.sum(self.geo['pore.all']) == array_sum
 
+    def test_setitem_subdict_conflicts(self):
+        self.geo['pore.foo'] = 1
+        with pytest.raises(Exception):
+            self.geo['pore.foo.bar'] = 1
+        self.geo['throat.foo.bar'] = 1
+        with pytest.raises(Exception):
+            self.geo['throat.foo'] = 1
+
     def test_object_name_name_conflict(self):
         with pytest.raises(Exception):
             self.geo.name = self.net.name
diff --git a/tests/unit/core/SubdomainTest.py b/tests/unit/core/SubdomainTest.py
index 0bd64a5f4..db183247e 100644
--- a/tests/unit/core/SubdomainTest.py
+++ b/tests/unit/core/SubdomainTest.py
@@ -74,6 +74,57 @@ class SubdomainTest:
         assert self.phase1.num_throats(self.phys1.name) == 0
         self.phys1.add_locations(pores=self.net.Ps, throats=self.net.Ts)
 
+    def test_writting_subdict_names_across_subdomains(self):
+        ws = op.Workspace()
+        proj = ws.new_project()
+
+        pn = op.network.Cubic(shape=[10, 10, 10], spacing=1e-4, project=proj)
+        Ps = pn['pore.coords'][:, 0] < pn['pore.coords'][:, 0].mean()
+        Ts = pn.find_neighbor_throats(pores=Ps, mode='xnor')
+        geo1 = op.geometry.StickAndBall(network=pn, pores=Ps, throats=Ts)
+
+        Ps = pn['pore.coords'][:, 0] >= pn['pore.coords'][:, 0].mean()
+        Ts = pn.find_neighbor_throats(pores=Ps, mode='or')
+        geo2 = op.geometry.StickAndBall(network=pn, pores=Ps, throats=Ts)
+
+        pn['pore.foo'] = 1
+        # Can't create a subdict below foo
+        with pytest.raises(Exception):
+            pn['pore.foo.bar'] = 1
+        # Can create a subdict directly
+        pn['pore.baz.bar'] = 2
+        # Can't create a new item already used as subdict
+        with pytest.raises(Exception):
+            pn['pore.baz'] = 2
+
+        # Also works on subdomains
+        geo1['pore.blah'] = 1
+        with pytest.raises(Exception):
+            geo1['pore.blah.boo'] = 1
+        geo1['pore.bee.bop'] = 1
+        with pytest.raises(Exception):
+            geo1['pore.bee'] = 1
+
+        # Now start looking across objects
+        with pytest.raises(Exception):
+            geo1['pore.foo'] = 1  # Already exists on pn
+        with pytest.raises(Exception):
+            geo1['pore.foo.bar'] = 1  # pore.foo already exists on pn
+        with pytest.raises(Exception):
+            geo1['pore.baz'] = 1  # pore.baz.bar already exists on pn
+
+        # Now start looking across objects
+        geo2['pore.blah'] = 1
+        geo2['pore.bee.bop'] = 1
+        with pytest.raises(Exception):
+            geo1['pore.bee'] = 1
+
+        with pytest.raises(Exception):
+            pn['pore.bee'] = 1
+
+        with pytest.raises(Exception):
+            pn['pore.bee.bop'] = 1
+
 
 if __name__ == '__main__':