Michael Bayer wrote: > my only concern is that you now have "more than one way to do it". i > need to deal with things in the identity map. do i go look at the > session.identity_map ? (which is documented, its part of the public > API) oh no, i dont have the exact kind of key to use, now i have to > go use a method called "find()" (which again, does that mean, find it > in the database ? where is it looking ?)
These are good points. Maybe the problem is in my brain--I've always had a disconnect between the session.identity_map and the mapper.identity_key() function. I guess it's clearly documented that they are compatible and can be used like this: key = MyClass.mapper.identity_key(pk_value) itm = session.identity_map.get(key) It just seemed like that was digging a bit too deep into what I thought were implementation details of the mapper and the session. If those things (i.e. mapper.identity_key and session.identity_map) are clearly documented as part of the interface of SA, and they are meant to work together like that then maybe this proposal isn't even necessary. After all, it's just two lines instead of one. However, upon looking at the documentation, this is all I find on the "identity_key" method of the Mapper class: def identity_key(self, primary_key) deprecated. a synonym for identity_key_from_primary_key. Now I thought identity_key was OK (if a bit obscure due to lack of documentation), but identity_key_from_primary_key is not so great IMHO. This is not a method name that will come to mind when I'm trying to get the identity key of a given object. It's just too long. Would it be OK to un-deprecate identity_key and just state clearly in the documentation that it requires a primary key as it's argument? Change it like this: def identity_key(self, pk=None, instance=None) """Return the identity key given a primary key OR an instance Either the pk or instance keyword argument must be supplied. An error will be raised if both instance and pk are given or if both are None. """ Note that this is backward-compatible with the previous version of identity_key, which took a primary key as its first argument. Then do this: identity_key_from_primary_key -> deprecated identity_key_from_instance -> deprecated Finally, we also need to clearly document in the section that talks about the identity_map that the keys used in that map may be obtained directly from the mapper of the object by using mapper.identity_key(). If those things were cleared up I would see no reason why we need a session.get_local() or session.find() or whatever... And we have one clear way to do it. What do you think of this? ~ Daniel --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
