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 -~----------~----~----~----~------~----~------~--~---
classstate_dict.patch
Description: Binary data