Michael Bayer wrote:
> id rather not have people needing to deal with an actual "identity
> key" tuple most of the time.  they should be able to say
> session.identity_map.something(class, pk) and get an instance  out of
> it.

What's the use of exposing the identity map if you don't want people to deal 
with the keys with which it's indexed? You might as well expose an 
identity_set, except that would be really unhandy as it would be expensive or 
impossible to do the exact kind of "find" that I'm asking for.

> 
> the reason for those big names on mapper is because all of them are
> used, we need identity keys from instances, rows, primary keys, all of
> it, and i was myself getting confused since their names were not
> clear...so i changed the names to be absolutely clear.  but also, i
> dont want people generally dealing with the methods off of
> Mappers...all the methods you need should be off of Session and
> Query.  so maybe we can put a keyword-oriented "identity_key" method
> on Session, or something (identity_key(primary_key=None,
> instance=None, row=None, etc)).  or maybe it can use some kind of
> "multiple-dispatch" that detects scalar, tuple, object instance,
> ResultProxy.  but also, i dont think the "get an SA identity key" step
> as an interim step to accomplishing something else should really be
> needed in the usual case.

If the session got an identity_key() method that would be great (and eliminate 
the need for a "find" method). Here's an implementation:

def identity_key(self, *args, **kwargs):
    """Get an identity key
    
    Valid call signatures:

    identity_key(class_, ident, entity_name=None)
        class_ - mapped class
        ident - primary key, if the key is composite this is a tuple
        entity_name - optional entity name. May be given as a
            positional arg or as a keyword arg.

    identity_key(instance=instance)
        instance - object instance (must be given as a keyword arg)
    
    identity_key(row=row)
        row - result proxy row (must be given as a keyword arg)
    """
    if args:
        kw = {}
        if len(args) == 2:
            class_, ident = args
            entity_name = kwargs.pop("entity_name", None)
            assert not kwargs, ("unknown keyword arguments: %s"
                % (kwargs.keys(),))
        else:
            assert len(args) == 3, ("two or three positional args are "
                "accepted, got %s" % len(args))
            class_, ident, entity_name = args
        mapper = _class_mapper(class_, entity_name=entity_name)
        return mapper.instance_key_from_primary_key(ident,
            entity_name=entity_name)
    else:
        try:
            instance = kwargs.pop("instance")
        except KeyError:
            row = kwargs.pop("row")
            assert not kwargs, ("unknown keyword arguments: %s"
                % (kwargs.keys(),))
            mapper = # not sure how to get the mapper form a row
            return mapper.identity_key_from_row(row)
        else:
            assert not kwargs, ("unknown keyword arguments: %s"
                % (kwargs.keys(),))
            mapper = _object_mapper(instance)
            return mapper.identity_key_from_instance(instance)


Note that I didn't implement the part to get a mapper from a row because I'm 
not sure how to do that. I imagine that's trivial though.

~ Daniel


> 
> On Mar 10, 8:27 pm, Daniel Miller <[EMAIL PROTECTED]> wrote:
>> 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
-~----------~----~----~----~------~----~------~--~---

Reply via email to