AssociationProxy should use keywords on create

[Marc Schlaich]

I want to use the Association Object pattern as described in the docs (http://docs.sqlalchemy.org/en/rel_0_8/orm/extensions/associationproxy.html#simplifying-association-objects). But I do not want to explicitly write a constructor and rely on the new "every attribute can be passed as kwarg" style.

This is currently broken with associationproxy, because it passes the proxied value directly to the constructor. So if the Association Object has no explicit constructor, the code fails:

Traceback (most recent call last):
  File "d:\Projekte\HACS\tests\test_model.py", line 12, in test_prog_order
    job.programs.append(j1)
  File "d:\Projekte\HACS\hacs\packages\sqlalchemy\ext\associationproxy.py", line 567, in append
    item = self._create(value)
  File "d:\Projekte\HACS\hacs\packages\sqlalchemy\ext\associationproxy.py", line 494, in _create
    return self.creator(value)
TypeError: __init__() takes exactly 1 argument (2 given)

Fix for me was to patch _AssociationList._create with the following, but maybe I'm missing something.

def _create(self, value):
    return self.creator(**{self.parent.value_attr: value})

If it looks good to you, I'll send a PR on GitHub to the 0.8 branch.

[Mike Bayer]

can you send a usage example please, also all new features go to master first and are backported to 0.8 as necessary.

[Mike Bayer]

particularly, why can't you use the "creator" argument? I'm not following.

[Marc Schlaich]

Testcase:

from sqlalchemy import Column, Integer, ForeignKey, String
from sqlalchemy.orm import relationship, backref

from sqlalchemy.ext.associationproxy import association_proxy
from sqlalchemy.ext.declarative import declarative_base

Base = declarative_base()


class User(Base):
    __tablename__ = 'user'
    id = Column(Integer, primary_key=True)
    name = Column(String(64))
    keywords = association_proxy('user_keywords', 'keyword')


class UserKeyword(Base):
    __tablename__ = 'user_keyword'
    user_id = Column(Integer, ForeignKey('user.id'), primary_key=True)
    keyword_id = Column(Integer, ForeignKey('keyword.id'), primary_key=True)

    # bidirectional attribute/collection of "user"/"user_keywords"
    user = relationship(User, backref=backref("user_keywords",
                        cascade="all, delete-orphan"))

    # reference to the "Keyword" object
    keyword = relationship("Keyword")


class Keyword(Base):
    __tablename__ = 'keyword'
    id = Column(Integer, primary_key=True)
    keyword = Column(String(64))


user = User(name='log')
for kw in (Keyword(keyword='new_from_blammo'), Keyword(keyword='its_big')):
    user.keywords.append(kw)

Error:

$ python sqla_2808.py
Traceback (most recent call last):
  File "sqla_2808.py", line 39, in <module>
    user.keywords.append(kw)
  File "D:\Projekte\HACS\hacs\packages\sqlalchemy\ext\associationproxy.py", line 567, in append
    item = self._create(value)
  File "D:\Projekte\HACS\hacs\packages\sqlalchemy\ext\associationproxy.py", line 494, in _create
    return self.creator(value)
TypeError: __init__() takes exactly 1 argument (2 given)

[Mike Bayer]

that's what "creator" is for:

class User(Base):
    __tablename__ = 'user'
    id = Column(Integer, primary_key=True)
    name = Column(String(64))
    keywords = association_proxy('user_keywords', 'keyword', creator=lambda v: UserKeyword(keyword=v))

this can be generalized with your preferred style:

def my_association_proxy(src, target):
    def create(value):
        target_cls = prox.target_class
        return target_cls(**{target: value})
    prox = association_proxy(src, target, creator=create)
    return prox

class User(Base):
    __tablename__ = 'user'
    id = Column(Integer, primary_key=True)
    name = Column(String(64))
    keywords = my_association_proxy('user_keywords', 'keyword')

it should be apparent why we can't just change _AssociationList._create() here.

[Mike Bayer]

docs and examples for "creator" are at http://docs.sqlalchemy.org/en/rel_0_8/orm/extensions/associationproxy.html#creation-of-new-values .

[Mike Bayer]

I'm going to close this for now but feel free to reopen if you have further issues with the current system.

[Marc Schlaich]

I think this is rather a workaround than a solution in case of an association object because it is really unintuitive.

From a user point of view the "association proxy" pattern shouldn't be this complex. Essentially you say: proxy this attribute as a collection via a many-to-many mapping. When a user pass an object to this collection, SQLAlchemy should know how to build the many-to-many relationship without requiring manual configuration from the user.

So IMO the creation process in case of an association object should behave something like this (with User/Keyword example)

def _create(keyword):
    # create association object (UserKeyword)
    # do not pass the value (or pass the value
    # but the default creator shouldn't pass it 
    # further to UserKeyword)
    uk = self.creator()
    # automatically find out how Keyword is related with 
    # UserKeyword and set the correct attribute
    uk.find_relationship_and_set(keyword)

[Marc Schlaich]

Plus, your example (with creator) doesn't work because UserKeyword is not yet defined for the User class. So you would have to refactor the complete example to make it work (which could be really messy if you have to use this solution in production code).

[Mike Bayer]

Replying to schlamar:

I think this is rather a workaround than a solution in case of an association object because it is really unintuitive.

From a user point of view the "association proxy" pattern shouldn't be this complex. Essentially you say: proxy this attribute as a collection via a many-to-many mapping. When a user pass an object to this collection, SQLAlchemy should know how to build the many-to-many relationship without requiring manual configuration from the user.

The reason we don't do things like that is because the user sees it as "magic", and when things go wrong, even simple things, they have absolutely no idea where to look. A many-to-many mapping has lots of options - the secondary table, the join condition, backrefs, it has an attribute name of its own, the user needs to specify these things explicitly.

The reason SQLAlchemy's patterns are so explicit is because they get intricate really fast, and we prefer the user to work out the individual links in the chain. There are always ways to automate these patterns, so the issue is not "too much typing" - you can automate that.

If you want to see the opposite, look at sqlalchemy-elixir, which hides everything and creates attribute names and all that behind the scenes. When something goes wrong with that, the user is lost. Hence declarative's explicit approach has proven to be more popular.

So IMO the creation process in case of an association object should behave something like this (with User/Keyword example)

{{{ #!python

def _create(keyword): # create association object (UserKeyword) # do not pass the value (or pass the value # but the default creator shouldn't pass it # further to UserKeyword) uk = self.creator() # automatically find out how Keyword is related with # UserKeyword and set the correct attribute uk.find_relationship_and_set(keyword)

}}}

This is possible for simple cases but then is not feasible for complex cases, so as the scenario grows more complex, the easy magic starts to make mistakes and we get bug reports. It's better to ask the user to explicitly tell us what to do rather than guessing.

[Mike Bayer]

Replying to schlamar:

Plus, your example (with creator) doesn't work because UserKeyword is not yet defined for the User class. So you would have to refactor the complete example to make it work (which could be really messy if you have to use this solution in production code).

The my_association_proxy() function doesn't need access to the UserKeyword class. Feel free to actually run the recipe in context of your original example:

from sqlalchemy import Column, Integer, ForeignKey, String
from sqlalchemy.orm import relationship, backref

from sqlalchemy.ext.associationproxy import association_proxy
from sqlalchemy.ext.declarative import declarative_base

Base = declarative_base()

def my_association_proxy(src, target):
    def create(value):
        target_cls = prox.target_class
        return target_cls(**{target: value})
    prox = association_proxy(src, target, creator=create)
    return prox

class User(Base):
    __tablename__ = 'user'
    id = Column(Integer, primary_key=True)
    name = Column(String(64))
    keywords = my_association_proxy('user_keywords', 'keyword')

class UserKeyword(Base):
    __tablename__ = 'user_keyword'
    user_id = Column(Integer, ForeignKey('user.id'), primary_key=True)
    keyword_id = Column(Integer, ForeignKey('keyword.id'), primary_key=True)

    # bidirectional attribute/collection of "user"/"user_keywords"
    user = relationship(User, backref=backref("user_keywords",
                        cascade="all, delete-orphan"))

    # reference to the "Keyword" object
    keyword = relationship("Keyword")


class Keyword(Base):
    __tablename__ = 'keyword'
    id = Column(Integer, primary_key=True)
    keyword = Column(String(64))


user = User(name='log')
for kw in (Keyword(keyword='new_from_blammo'), Keyword(keyword='its_big')):
    user.keywords.append(kw)

print(user.keywords)

[Mike Bayer]

a post regarding the "explicit, but you can automate it!" nature of SQLAlchemy is here:

http://techspot.zzzeek.org/2011/05/17/magic-a-new-orm/

as far as "creator", I will grant that given the nature of Declarative (which came after association proxy), it would have been better if it assumed keyword arguments to start with, rather than positional. I'd be willing to examine an approach that uses inspect() to make simple deductions here as to if named arguments can be used.

[Mike Bayer]

if the proposal is to have association proxy generate "relationship()" automatically, we can't do that. if the proposal is to have the implicit "creator" do kw arguments instead of positional, I think it's too late to change it like that and simple recipes can work around it. I'd accept more documentation here.

[Mike Bayer]

Removing milestone: 0.8.xx (automated comment)