At 04:04 PM 2/27/2008 -0500, Michael Bayer wrote:
>do you have any interest in committing changes to the branch
>yourself ?  as long as the unit tests keep running whatever you'd want
>is most likely fine with me....otherwise I will at least experiment
>with doing away with __mro__ searching and possibly doing away with
>pre_instrument_attribute.

Okay, I've run into an interesting hitch on the branch -- I think 
it's a test coverage issue.  The definition of 
'ClassState.is_instrumented' is different from that of 
_ClassStateAdapter, in a way that appears to *matter*.  That is, if 
you change ClassState.is_instrumented to work in roughly the same way 
as _ClassStateAdapter.is_instrumented, the standard tests 
fail.  Specifically, with this rather unhelpful error...  :)

Traceback (most recent call last):
   File "test/orm/attributes.py", line 397, in test_collectionclasses
     assert False
AssertionError

This would appear to indicate that there is some kind of hidden 
dependency between collections support, and the presence of SA 
descriptors on a class.

Further investigation reveals that the problem comes from 
register_attribute() checking to see if an attribute is already 
instrumented, and if so, returns immediately.  It would appear that 
this really wants to know if there is actually an instrumented 
descriptor, rather than whether one has merely been registered with 
the class state, per these comments::

# this currently only occurs if two primary mappers are made for the 
same class.
# TODO:  possibly have InstrumentedAttribute check "entity_name" when 
searching for impl.
# raise an error if two attrs attached simultaneously otherwise

I'm pretty much stumped at this point.  Essentially, even though the 
tests don't cover this point for custom instrumentation, it means 
that this problem *will* occur with custom instrumentation, even 
without my changes.

Now, the *good* news is, the other 1190 tests all pass with my changes.  :)

I've attached my current diff against the user_defined_state 
branch.  Note that this bit is the part that makes the one test break:

      def is_instrumented(self, key, search=False):
          if search:
-            return hasattr(self.class_, key) and 
isinstance(getattr(self.class_, key), InstrumentedAttribute)
+            return key in self
          else:
-            return key in self.class_.__dict__ and 
isinstance(self.class_.__dict__[key], InstrumentedAttribute)
+            return key in self.local_attrs

In other words, it's only the change to ClassState.is_instrumented 
that breaks the test, showing that there's a meaningful difference 
here between what gets registered in the state, and what's actually 
on the class.  Which is going to be a problem for anybody rerouting 
the descriptors (the way the Trellis library will be).

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"sqlalchemy" group.
To post to this group, send email to sqlalchemy@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/sqlalchemy?hl=en
-~----------~----~----~----~------~----~------~--~---

Attachment: classstate_dict.patch
Description: Binary data

Reply via email to