At 02:04 PM 3/3/2008 -0500, Michael Bayer wrote:

>the bug is that unregister_attribute() is not working, which the test
>suite is using to remove and re-register new instrumentation:
>
>         class Foo(object):
>                 pass
>
>          attributes.register_attribute(Foo, "collection",
>uselist=True, typecallable=set, useobject=True)
>          assert
>attributes.get_class_state(Foo).is_instrumented("collection")
>          attributes.unregister_attribute(Foo, "collection")
>          assert not
>attributes.get_class_state(Foo).is_instrumented("collection")
>
>seems like unregister_attribute() is still doing the old thing of just
>"del class.attr", so we'd just need to stick a hook similar to
>"instrument_attribute" for this on ClassState which will take over the
>job.
>
>looks good !

Okay, so I did a matching uninstrument_attribute and 
pre_uninstrument_attribute, which look like really dumb names at this 
point.  I propose the following name changes (in addition to the ones 
in my previous patch):

pre_instrument_attribute -> install_descriptor
pre_uninstrument_attribute -> uninstall_descriptor

That okay?  instrument_attribute will then call install_descriptor to 
do the actual installing.  And of course the hooks on the adapted 
thingy from the class would work the same way.

If that's okay with you, then after I'm done I'll post a patch for 
review before checkin.  After that, I'll start work on the 
Trellis-side support for this, and then eventually dig into  collections stuff.


--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---

Reply via email to