[sqlalchemy] Re: Use of new_instance() in the user-defined-state branch?
At 09:45 AM 3/28/2008 -0400, Michael Bayer wrote: On Mar 28, 2008, at 12:55 AM, Phillip J. Eby wrote: Sadly, about the only way for me to implement that without code duplication will be to temporarily change the item's __class__ to a subclass with an empty __init__ method. Unless there's a way to change the generated __init__ method to take an extra flag or check a per-thread variable to skip the bits you don't want? What are the bits you don't want run, anyway? That is, what specifically mustn't happen? an end-user's __init__ method is not run when the object is loaded from the DB since the ORM is going to populate its state explicitly. its for similar reasons that pickle.loads() doesn't call __init__. Its a common use case that someone's class needs to be constructed differently when it is newly created vs. when it is re-populated from the DB. The usual method we have of modifying this behavior is to use a MapperExtension where you implement create_instance(). Then you can build the object any way you want, using __init__, whatever. Why is that not an option here ? ( or has it just not been mentioned ?) Because Jason said this: At 06:08 PM 3/27/2008 -0700, jason kirtland wrote: Phillip J. Eby wrote: At 02:26 PM 3/27/2008 -0700, jason kirtland wrote: new_instance creates an instance without invoking __init__. The ORM uses it to recreate instances when loading from the database. new_instance can be added to InstrumentationManager as an extension method... The ORM doesn't care how empty instances are manufactured so long as they can be created without initialization arguments, e.g. a no-arg constructor. Does that mean that no attributes must be set from new_instance(), either? You should be able to set whatever you like there. So... new_instance() could literally just be a call to 'self.class_()', with no other behavior, as long as the constructor requires no arguments? The modified __init__ that SA inserts won't be a problem there? Sorry, I should have included more detail. You don't want to trigger the logic in SA's __init__ decorator or call the user's __init__. The ORM policy is not to __init__ on load, and will soon support a symmetric __on_load__ type of hook that the ORM can call post-__new__ when reconstituting instances. So, which one of you is right? :) --~--~-~--~~~---~--~~ 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 -~--~~~~--~~--~--~---
[sqlalchemy] Re: Use of new_instance() in the user-defined-state branch?
So you're still disagreeing with Jason, who's quite explicitly saying that SA's __init__ will blow up if it gets called. Which of you is right? :) At 11:38 AM 3/28/2008 -0400, Michael Bayer wrote: On Mar 28, 2008, at 10:58 AM, Phillip J. Eby wrote: Sorry, I should have included more detail. You don't want to trigger the logic in SA's __init__ decorator or call the user's __init__. The ORM policy is not to __init__ on load, and will soon support a symmetric __on_load__ type of hook that the ORM can call post-__new__ when reconstituting instances. So, which one of you is right? :) Well, I'm not entirely sure how your users will be using their objects. If they just want to take any old application and enable trellis + sqlalchemy, if they are accustomed to writing for SA then it would be a surprise for their __init__() method to be called. Like, if I wrote a class like this: class MyClass(object): def __init__(self, a, b): self.a = a self.b = b then I mapped it to Trellis + SQLA, we *can't* call MyClass() upon load from the database - we dont have the constructor arguments available and TypeError will be thrown. If OTOH, using Trellis implies that you must already have an __init__() that is compatible with a no-arg calling style and that they should expect population of attributes to occur after it's called, then theres no issue with configuring the SA mappings to call __init__(). I think you mentioned earlier that Trellis doesn't care what the user does with __init__()neither does SQLAlchemy, and thats why we never call it by default with no args, since we make no assumptions about what it expects or what it does. --~--~-~--~~~---~--~~ 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 -~--~~~~--~~--~--~---
[sqlalchemy] Use of new_instance() in the user-defined-state branch?
I just noticed that in the latest version of the branch, there's a new_instance() call that is using a class' __new__ method in order to create a new instance, rather than using 'class()'. What I'd like to find out is how to get around this, because Trellis objects will not be properly initialized unless the 'class()' is called, with any initialization taking place inside __new__ and/or __init__. Trellis doesn't override __new__ or __init__, and doesn't care what they do. But the creation of an instance *must* be wrapped by the class' __call__ (i.e. class()), as there is a try/finally involved that must execute. Any thoughts on how this might be refactored? What is new_instance() used for? --~--~-~--~~~---~--~~ 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 -~--~~~~--~~--~--~---
[sqlalchemy] Re: Use of new_instance() in the user-defined-state branch?
At 02:26 PM 3/27/2008 -0700, jason kirtland wrote: new_instance creates an instance without invoking __init__. The ORM uses it to recreate instances when loading from the database. new_instance can be added to InstrumentationManager as an extension method... The ORM doesn't care how empty instances are manufactured so long as they can be created without initialization arguments, e.g. a no-arg constructor. Does that mean that no attributes must be set from new_instance(), either? You should be able to set whatever you like there. So... new_instance() could literally just be a call to 'self.class_()', with no other behavior, as long as the constructor requires no arguments? The modified __init__ that SA inserts won't be a problem there? --~--~-~--~~~---~--~~ 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 -~--~~~~--~~--~--~---
[sqlalchemy] Re: Integrating the ORM with Trellis
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 -~--~~~~--~~--~--~---
[sqlalchemy] Re: Integrating the ORM with Trellis
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 meotherwise 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
