cleanup around _copyOfObject, and remove copyfunc argument

author: Bruce Smith <bruce@nanorex.com> 2008-12-29 23:03:06 +0000
committer: Bruce Smith <bruce@nanorex.com> 2008-12-29 23:03:06 +0000
commit: a4942a5fa76913dc477485d411db195bd63370a4 (patch)
tree: 555d8aa6926131c7358f4474d303ed10d2fcb72a
parent: 15eccfcb449e7439d0413c8542be60bca8b44547 (diff)
download: nanoengineer-theirix-a4942a5fa76913dc477485d411db195bd63370a4.tar.gz
nanoengineer-theirix-a4942a5fa76913dc477485d411db195bd63370a4.zip
8 files changed, 48 insertions, 39 deletions
diff --git a/cad/src/analysis/ESP/ESPImage.py b/cad/src/analysis/ESP/ESPImage.py
index 35419c654..9ff5ae409 100755
--- a/cad/src/analysis/ESP/ESPImage.py
+++ b/cad/src/analysis/ESP/ESPImage.py
@@ -785,8 +785,7 @@ class image_mod_record(DataMixin): #bruce 060210; maybe should be refiled in Ima
         return not not (self.mirrorQ or self.rot) # only correct since we always canonicalize rot by % 360
 
     # override abstract method of DataMixin
-    def _copyOfObject(self, copyfunc): # (in class image_mod_record [bruce circa 060210])
-        # ignores copyfunc
+    def _copyOfObject(self): # (in class image_mod_record [bruce circa 060210])
         return self.__class__(self.mirrorQ, self.rot)
 
     # override abstract method of DataMixin
diff --git a/cad/src/analysis/GAMESS/jig_Gamess.py b/cad/src/analysis/GAMESS/jig_Gamess.py
index bda7c166b..87b32fb02 100755
--- a/cad/src/analysis/GAMESS/jig_Gamess.py
+++ b/cad/src/analysis/GAMESS/jig_Gamess.py
@@ -652,8 +652,7 @@ class gamessParms(state_utils.DataMixin): #bruce 060306 added superclass
         return new
 
     # override abstract method of DataMixin
-    def _copyOfObject(self, copyfunc): #bruce 051003, for use by state_utils.copy_val
-        # ignores copyfunc
+    def _copyOfObject(self): #bruce 051003, for use by state_utils.copy_val
         return self.deepcopy(alter_name = False) ###k I'm not sure alter_name = False can ever be legal,
             # or (if it can be) whether it's good here. I think Mark or I should review this,
             # and we should not change the code to rely on copy_val alone on this object
diff --git a/cad/src/cnt/model/NanotubeParameters.py b/cad/src/cnt/model/NanotubeParameters.py
index fd6d0058c..fdd02785a 100644
--- a/cad/src/cnt/model/NanotubeParameters.py
+++ b/cad/src/cnt/model/NanotubeParameters.py
@@ -748,7 +748,7 @@ class NanotubeParameters:
         return
     
     # override abstract method of DataMixin
-    def _copyOfObject(self, copyfunc):
+    def _copyOfObject(self):
         """
         Create and return a copy of nanotube.
         """
diff --git a/cad/src/exprs/scratch/test_animation_mode.py b/cad/src/exprs/scratch/test_animation_mode.py
index 9b9476130..2578a0123 100644
--- a/cad/src/exprs/scratch/test_animation_mode.py
+++ b/cad/src/exprs/scratch/test_animation_mode.py
@@ -377,7 +377,7 @@ class _S_ImmutableData_Mixin(_S_Data_Mixin):
     copied as themselves. (Most of them deepcopy the data passed into them, to protect
     themselves from callers who might pass shared data.)
     """
-    def _s_deepcopy(self, copyfunc): ##k API
+    def _s_deepcopy(self, copyfunc): ##k API [not presently called as of 081229, AFAIK]
         #e maybe someday we'll inherit this from (say) _S_ImmutableData_Mixin
         return self
     def _s_copy_for_shallow_mod(self): #e likely to be renamed, maybe ...private_mod
diff --git a/cad/src/foundation/state_constants.py b/cad/src/foundation/state_constants.py
index ddbb3b5d1..70330d61c 100755
--- a/cad/src/foundation/state_constants.py
+++ b/cad/src/foundation/state_constants.py
@@ -100,7 +100,7 @@ class _UNSET_class:
         self.name = name
     def __repr__(self):
         return self.name
-    def _copyOfObject(self, copyfunc): # copied from IdentityCopyMixin
+    def _copyOfObject(self): # copied from IdentityCopyMixin
         return self
     def _isIdentityCopyMixin(self): # copied from IdentityCopyMixin
         pass
diff --git a/cad/src/foundation/state_utils.py b/cad/src/foundation/state_utils.py
index d3d4cdc30..98d0b4f63 100755
--- a/cad/src/foundation/state_utils.py
+++ b/cad/src/foundation/state_utils.py
@@ -41,19 +41,25 @@ DEBUG_PYREX_ATOMS = debug_pyrex_atoms()
 '''
 Where is _copyOfObject (etc) documented? (In code and on wiki?)
 
-That should say:
+On wiki: 
 
-- When defining _copyOfObject, consider:
+http://www.nanoengineer-1.net/mediawiki/index.php?title=How_to_add_attributes_to_a_model_object_in_NE1
+
+That documentation should say:
 
-  - is it correct for any copyfunc argument? (esp in its assumption
-    about what that returns, original or copy or transformed copy)
+- When defining _copyOfObject, consider:
   
   - is its own return value __eq__ to the original? It should be,
     so you have to define __eq__ AND __ne__ accordingly.
+    [later: I think the default def of __ne__ from __eq__ means
+     you needn't define your own __ne__.]
   
-  - should you define _s_scan_children to, to scan the same things copied?
+  - should you define _s_scan_children too, to scan the same things
+    that are copied?
     (Only if they are instance objects, and are "children".
      See S_CHILDREN doc for what that means.)
+    [later: I think this only matters if they can contain undoable state
+     or point to other objects which can.]
 
 I did the above for VQT and jigs_planes, but still no __eq__ or children
 for jig_Gamess -- I'll let that be a known bug I fix later,
@@ -708,13 +714,14 @@ def copy_InstanceType(obj): #e pass copy_val as an optional arg?
     ##if copier is not None:
     ##    return copier(obj, copy_val) # e.g. for QColor
     try:
-        # note: not compatible with copy.deepcopy's __deepcopy__ method
+        # note: not compatible with copy.deepcopy's __deepcopy__ method.
         # see DataMixin and IdentityCopyMixin below.
         copy_method = obj._copyOfObject 
     except AttributeError:
         print "****************** needs _copyOfObject: %s" % repr(obj)
         return obj
-    res = copy_method( copy_val)
+    res = copy_method() 
+        #bruce 081229 no longer pass copy_val (removed never-used copyfunc arg)
     if _debug_copyOfObject and (obj != res or (not (obj == res))): #bruce 060311 adding _debug_copyOfObject as optim (suggested by Will)
         # Bug in copy_method, which will cause false positives in change-detection in Undo (since we'll return res anyway).
         # (It's still better to return res than obj, since returning obj could cause Undo to completely miss changes.)
@@ -1879,7 +1886,7 @@ class obj_classifier:
 # ==
 
 class IdentityCopyMixin: # by EricM
-    def _copyOfObject(self, copyfunc):
+    def _copyOfObject(self):
         """
         Implements the copying of an object for copy_val.  For objects
         which care about their identity (which inherit from
@@ -1937,47 +1944,48 @@ class DataMixin:
     """
     Convenience mixin for classes that act as 'data' when present in
     values of declared state-holding attributes. Provides method stubs
-    to remind you when you haven't declared a necessary method.  Makes
+    to remind you when you haven't defined a necessary method. Makes
     sure state system treats this object as data (and doesn't warn
-    about it).  All such data-like classes which may be handled by
+    about it). All such data-like classes which may be handled by
     copy_val must inherit DataMixin.
     """
-    def _copyOfObject(self, copyfunc):
+    def _copyOfObject(self):
         """
-        Implements the copying of an object for copy_val.  For data
+        This method must be defined in subclasses to implement
+        the copying of an object for copy_val. For data
         objects (which inherit from DataMixin, or define
-        _s_isPureData), this should return a fresh object which will
-        compare __eq__ to the original, but which will have a
-        different id().  Implementation of this method must be
+        _s_isPureData), this should return a newly allocated object
+        which will be __eq__ to the original, but which will have a
+        different id(). Implementation of this method must be
         compatible with the implementation of __eq__ for this class.
 
-        This method is declared private solely for performance
-        reasons.  In particular, InvalMixin.__getattr__() has a fast
-        return for attributes which start with an underscore.  Many
+        This method has a name which appears private, solely for performance
+        reasons. In particular, InvalMixin.__getattr__() has a fast
+        return for attributes which start with an underscore. Many
         objects (like atoms) inherit from InvalMixin, and looking up
         non-existent attributes on them takes significantly longer if
-        the attribute name does not start with underscore.  In
+        the attribute name does not start with underscore. In
         general, such objects should inherit from IdentityCopyMixin as
         well, and thus have _copyOfObject defined in order to avoid
         exception processing overhead in copy_InstanceType(), so it
-        doesn't really matter.  Should something slip through the
+        doesn't really matter. Should something slip through the
         cracks, at least we're only imposing one slowdown on the copy,
         and not two.
-
-        It appears that copyfunc is never used, but could be needed
-        for a complex structure.
         """
         print "_copyOfObject needs to be overridden in", self
         print "  (implem must be compatible with __eq__)"
         return self
-    def _s_isPureData(self): # note: presence of this method makes sure this object is treated as data.
+    def _s_isPureData(self): 
+        # note: presence of this method makes sure this object is treated as data.
         pass
     def __eq__(self, other):
         print "__eq__ needs to be overridden in", self
-        print "  (implem must be compatible with _copyOfObject; don't forget to avoid '==' when comparing Numeric arrays)"
+        print "  (implem must be compatible with _copyOfObject; " \
+              "don't forget to avoid '==' when comparing Numeric arrays)"
         return self is other
     def __ne__(self, other):
-        return not (self == other) # this uses the __eq__ above, or one which the main class defined
+        return not (self == other) 
+            # this uses the __eq__ above, or one which the specific class defined
     pass
 
 # ===
diff --git a/cad/src/geometry/VQT.py b/cad/src/geometry/VQT.py
index d0403af13..f4a9ef5b4 100755
--- a/cad/src/geometry/VQT.py
+++ b/cad/src/geometry/VQT.py
@@ -353,14 +353,17 @@ class Q(DataMixin):
             return mat
         raise AttributeError, 'No %r in Quaternion' % (attr,)
 
-    #bruce 060209 defining __eq__ and __ne__ for efficient state comparisons given presence of __getattr__ (desirable for Undo)
-    # (I don't think it needs a __nonzero__ method, and if it had one I don't know if Q(1,0,0,0) should be False or True.)
-    #bruce 060222 note that it also now needs __eq__ and __ne__ to be compatible with its _copyOfObject (they are).
+    #bruce 060209 defining __eq__ and __ne__ for efficient state comparisons
+    # given presence of __getattr__ (desirable for Undo).
+    # (I don't think it needs a __nonzero__ method, and if it had one 
+    # I don't know if Q(1,0,0,0) should be False or True.)
+    #bruce 060222 note that it also now needs __eq__ and __ne__ to be 
+    # compatible with its _copyOfObject (they are).
     # later, __ne__ no longer needed since defined in DataMixin.
 
     # override abstract method of DataMixin
-    def _copyOfObject(self, copyfunc): #bruce 051003, for use by state_utils.copy_val (in class Q)
-        # ignores copyfunc
+    def _copyOfObject(self): 
+        #bruce 051003, for use by state_utils.copy_val (in class Q)
         return self.__class__(self)
     
     # override abstract method of DataMixin
diff --git a/cad/src/protein/model/Protein.py b/cad/src/protein/model/Protein.py
index d61f09df4..e5b8c91d7 100644
--- a/cad/src/protein/model/Protein.py
+++ b/cad/src/protein/model/Protein.py
@@ -648,7 +648,7 @@ class Protein:
         return
     
     # override abstract method of DataMixin
-    def _copyOfObject(self, copyfunc):
+    def _copyOfObject(self):
         """
         Create and return a copy of protein.
         """
author	Bruce Smith <bruce@nanorex.com>	2008-12-29 23:03:06 +0000
committer	Bruce Smith <bruce@nanorex.com>	2008-12-29 23:03:06 +0000
commit	a4942a5fa76913dc477485d411db195bd63370a4 (patch)
tree	555d8aa6926131c7358f4474d303ed10d2fcb72a
parent	15eccfcb449e7439d0413c8542be60bca8b44547 (diff)
download	nanoengineer-theirix-a4942a5fa76913dc477485d411db195bd63370a4.tar.gz nanoengineer-theirix-a4942a5fa76913dc477485d411db195bd63370a4.zip