more cleanup

3 files changed, 77 insertions, 46 deletions

diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py
index 57b4ee8bf..a51a94e5c 100644
--- a/lib/sqlalchemy/orm/attributes.py
+++ b/lib/sqlalchemy/orm/attributes.py
@@ -129,7 +129,8 @@ class QueryableAttribute(interfaces.PropComparator):
         try:
             return getattr(self.comparator, key)
         except AttributeError:
-            raise AttributeError('Neither %r object nor %r object has an attribute %r' % (
+            raise AttributeError(
+                    'Neither %r object nor %r object has an attribute %r' % (
                     type(self).__name__, 
                     type(self.comparator).__name__, 
                     key)
@@ -212,7 +213,8 @@ def proxied_attribute_factory(descriptor):
                 try:
                     return getattr(self._comparator, attribute)
                 except AttributeError:
-                    raise AttributeError('Neither %r object nor %r object has an attribute %r' % (
+                    raise AttributeError(
+                            'Neither %r object nor %r object has an attribute %r' % (
                             type(descriptor).__name__, 
                             type(self._comparator).__name__, 
                             attribute)
@@ -230,8 +232,8 @@ class AttributeImpl(object):
 
     def __init__(self, class_, key,
                     callable_, trackparent=False, extension=None,
-                    compare_function=None, active_history=False, parent_token=None, 
-                    expire_missing=True,
+                    compare_function=None, active_history=False, 
+                    parent_token=None, expire_missing=True,
                     **kwargs):
         """Construct an AttributeImpl.
 
@@ -293,7 +295,8 @@ class AttributeImpl(object):
         self.expire_missing = expire_missing
         
     def hasparent(self, state, optimistic=False):
-        """Return the boolean value of a `hasparent` flag attached to the given item.
+        """Return the boolean value of a `hasparent` flag attached to 
+        the given state.
 
         The `optimistic` flag determines what the default return value
         should be if no `hasparent` flag can be located.
@@ -332,10 +335,7 @@ class AttributeImpl(object):
         ``InstrumentedAttribute`` constructor.
 
         """
-        if callable_ is None:
-            self.initialize(state)
-        else:
-            state.callables[self.key] = callable_
+        state.callables[self.key] = callable_
 
     def get_history(self, state, dict_, passive=PASSIVE_OFF):
         raise NotImplementedError()
@@ -349,7 +349,7 @@ class AttributeImpl(object):
             return None
 
     def initialize(self, state, dict_):
-        """Initialize this attribute on the given object instance with an empty value."""
+        """Initialize the given state's attribute with an empty value."""
 
         dict_[self.key] = None
         return None
@@ -478,11 +478,16 @@ class MutableScalarAttributeImpl(ScalarAttributeImpl):
     def __init__(self, class_, key, callable_,
                     class_manager, copy_function=None,
                     compare_function=None, **kwargs):
-        super(ScalarAttributeImpl, self).__init__(class_, key, callable_,
-                                compare_function=compare_function, **kwargs)
+        super(ScalarAttributeImpl, self).__init__(
+                                            class_, 
+                                            key, 
+                                            callable_,
+                                            compare_function=compare_function, 
+                                            **kwargs)
         class_manager.mutable_attributes.add(key)
         if copy_function is None:
-            raise sa_exc.ArgumentError("MutableScalarAttributeImpl requires a copy function")
+            raise sa_exc.ArgumentError(
+                        "MutableScalarAttributeImpl requires a copy function")
         self.copy = copy_function
 
     def get_history(self, state, dict_, passive=PASSIVE_OFF):
@@ -495,7 +500,9 @@ class MutableScalarAttributeImpl(ScalarAttributeImpl):
             self, state, v)
 
     def check_mutable_modified(self, state, dict_):
-        (added, unchanged, deleted) = self.get_history(state, dict_, passive=PASSIVE_NO_INITIALIZE)
+        added, \
+        unchanged, \
+        deleted = self.get_history(state, dict_, passive=PASSIVE_NO_INITIALIZE)
         return bool(added or deleted)
 
     def get(self, state, dict_, passive=PASSIVE_OFF):
@@ -525,9 +532,11 @@ class MutableScalarAttributeImpl(ScalarAttributeImpl):
 
 
 class ScalarObjectAttributeImpl(ScalarAttributeImpl):
-    """represents a scalar-holding InstrumentedAttribute, where the target object is also instrumented.
+    """represents a scalar-holding InstrumentedAttribute, 
+       where the target object is also instrumented.
 
-    Adds events to delete/set operations.
+       Adds events to delete/set operations.
+       
     """
 
     accepts_scalar_loader = False
@@ -536,9 +545,14 @@ class ScalarObjectAttributeImpl(ScalarAttributeImpl):
     def __init__(self, class_, key, callable_, 
                     trackparent=False, extension=None, copy_function=None,
                     compare_function=None, **kwargs):
-        super(ScalarObjectAttributeImpl, self).__init__(class_, key,
-          callable_, trackparent=trackparent, extension=extension,
-          compare_function=compare_function, **kwargs)
+        super(ScalarObjectAttributeImpl, self).__init__(
+                                                class_, 
+                                                key,
+                                                callable_, 
+                                                trackparent=trackparent, 
+                                                extension=extension,
+                                                compare_function=compare_function, 
+                                                **kwargs)
         if compare_function is None:
             self.is_equal = identity_equal
 
@@ -621,17 +635,19 @@ class CollectionAttributeImpl(AttributeImpl):
     def __init__(self, class_, key, callable_, 
                     typecallable=None, trackparent=False, extension=None,
                     copy_function=None, compare_function=None, **kwargs):
-        super(CollectionAttributeImpl, self).__init__(class_, key, callable_, trackparent=trackparent,
-          extension=extension, compare_function=compare_function, **kwargs)
+        super(CollectionAttributeImpl, self).__init__(
+                                                class_, 
+                                                key, 
+                                                callable_, 
+                                                trackparent=trackparent,
+                                                extension=extension,
+                                                compare_function=compare_function, 
+                                                **kwargs)
 
         if copy_function is None:
             copy_function = self.__copy
         self.copy = copy_function
-
         self.collection_factory = typecallable
-        # may be removed in 0.5:
-        self.collection_interface = \
-          util.duck_type_collection(self.collection_factory())
 
     def __copy(self, item):
         return [y for y in list(collections.collection_adapter(item))]
@@ -822,29 +838,46 @@ class GenericBackrefExtension(interfaces.AttributeExtension):
     def set(self, state, child, oldchild, initiator):
         if oldchild is child:
             return child
+            
         if oldchild is not None and oldchild is not PASSIVE_NO_RESULT:
             # With lazy=None, there's no guarantee that the full collection is
             # present when updating via a backref.
             old_state, old_dict = instance_state(oldchild), instance_dict(oldchild)
             impl = old_state.get_impl(self.key)
             try:
-                impl.remove(old_state, old_dict, state.obj(), initiator, passive=PASSIVE_NO_FETCH)
+                impl.remove(old_state, 
+                            old_dict, 
+                            state.obj(), 
+                            initiator, passive=PASSIVE_NO_FETCH)
             except (ValueError, KeyError, IndexError):
                 pass
+                
         if child is not None:
-            new_state,  new_dict = instance_state(child), instance_dict(child)
-            new_state.get_impl(self.key).append(new_state, new_dict, state.obj(), initiator, passive=PASSIVE_NO_FETCH)
+            child_state, child_dict = instance_state(child), instance_dict(child)
+            child_state.get_impl(self.key).append(
+                                            child_state, 
+                                            child_dict, 
+                                            state.obj(), 
+                                            initiator, passive=PASSIVE_NO_FETCH)
         return child
 
     def append(self, state, child, initiator):
         child_state, child_dict = instance_state(child), instance_dict(child)
-        child_state.get_impl(self.key).append(child_state, child_dict, state.obj(), initiator, passive=PASSIVE_NO_FETCH)
+        child_state.get_impl(self.key).append(
+                                            child_state, 
+                                            child_dict, 
+                                            state.obj(), 
+                                            initiator, passive=PASSIVE_NO_FETCH)
         return child
 
     def remove(self, state, child, initiator):
         if child is not None:
             child_state, child_dict = instance_state(child), instance_dict(child)
-            child_state.get_impl(self.key).remove(child_state, child_dict, state.obj(), initiator, passive=PASSIVE_NO_FETCH)
+            child_state.get_impl(self.key).remove(
+                                            child_state, 
+                                            child_dict, 
+                                            state.obj(), 
+                                            initiator, passive=PASSIVE_NO_FETCH)
 
 
 class Events(object):
@@ -1295,13 +1328,6 @@ class History(tuple):
 
 HISTORY_BLANK = History(None, None, None)
 
-def _conditional_instance_state(obj):
-    if not isinstance(obj, state.InstanceState):
-        obj = instance_state(obj)
-    else:
-        util.warn_deprecated("Passing an InstanceState to get_history() or init_collection() is deprecated.")
-    return obj
-        
 def get_history(obj, key, **kwargs):
     """Return a History record for the given object and attribute key.
     
@@ -1310,7 +1336,7 @@ def get_history(obj, key, **kwargs):
     this usage is deprecated.
     
     """
-    return get_state_history(_conditional_instance_state(obj), key, **kwargs)
+    return get_state_history(instance_state(obj), key, **kwargs)
 
 def get_state_history(state, key, **kwargs):
     return state.get_history(key, **kwargs)
@@ -1408,7 +1434,7 @@ def init_collection(obj, key):
     this usage is deprecated.
     
     """
-    state = _conditional_instance_state(obj)
+    state = instance_state(obj)
     dict_ = state.dict
     return init_state_collection(state, dict_, key)
     
diff --git a/lib/sqlalchemy/util.py b/lib/sqlalchemy/util.py
index 9f9235450..07b8269a9 100644
--- a/lib/sqlalchemy/util.py
+++ b/lib/sqlalchemy/util.py
@@ -498,10 +498,11 @@ def class_hierarchy(cls):
     return list(hier)
 
 def iterate_attributes(cls):
-    """iterate all the keys and attributes associated with a class, without using getattr().
+    """iterate all the keys and attributes associated 
+       with a class, without using getattr().
 
-    Does not use getattr() so that class-sensitive descriptors (i.e. property.__get__())
-    are not called.
+       Does not use getattr() so that class-sensitive 
+       descriptors (i.e. property.__get__()) are not called.
 
     """
     keys = dir(cls)
@@ -600,9 +601,13 @@ def assert_arg_type(arg, argtype, name):
         return arg
     else:
         if isinstance(argtype, tuple):
-            raise exc.ArgumentError("Argument '%s' is expected to be one of type %s, got '%s'" % (name, ' or '.join("'%s'" % str(a) for a in argtype), str(type(arg))))
+            raise exc.ArgumentError(
+                            "Argument '%s' is expected to be one of type %s, got '%s'" % 
+                            (name, ' or '.join("'%s'" % a for a in argtype), type(arg)))
         else:
-            raise exc.ArgumentError("Argument '%s' is expected to be of type '%s', got '%s'" % (name, str(argtype), str(type(arg))))
+            raise exc.ArgumentError(
+                            "Argument '%s' is expected to be of type '%s', got '%s'" % 
+                            (name, argtype, type(arg)))
 
 _creation_order = 1
 def set_creation_order(instance):
diff --git a/test/aaa_profiling/test_orm.py b/test/aaa_profiling/test_orm.py
index 7bdc485ba..3027d4a50 100644
--- a/test/aaa_profiling/test_orm.py
+++ b/test/aaa_profiling/test_orm.py
@@ -58,7 +58,7 @@ class MergeTest(_base.MappedTest):
         # down from 185 on this
         # this is a small slice of a usually bigger
         # operation so using a small variance
-        @profiling.function_call_count(91, variance=0.001)
+        @profiling.function_call_count(95, variance=0.001)
         def go():
             return sess2.merge(p1, load=False)
             
@@ -66,7 +66,7 @@ class MergeTest(_base.MappedTest):
 
         # third call, merge object already present.
         # almost no calls.
-        @profiling.function_call_count(10, variance=0.001)
+        @profiling.function_call_count(12, variance=0.001)
         def go():
             return sess2.merge(p2, load=False)