On Feb 10, 2014, at 2:36 PM, Chris Withers <ch...@simplistix.co.uk> wrote:
> Hi All,
>
> I'm trying to be efficient with my code, but it seems to tripping SQLAlchemy
> up.
>
> So, basically I want to have a schema with two tables, content and project,
> to represent two three types of object:
if you poke around with print statements or pdb, you can see that the “id”
declared_attr is run only once, for Content. this is because Content is
mapped and your “id” declared_attr function is replaced with the mapped
attribute “id”.
If you make a mixin like HasId and put it there, it still doesn’t work.
Declarative finds the HasId mixin and the “id” function, but then when it
resolves what “Project.id” really is, it again just calls getattr() and doesn’t
call your function.
Why’s it like that? I don’t know, but I’m 99% sure at least three tests will
fail if I change it, lets try:
Code like this:
class HasId(object):
@declared_attr
def id(cls):
if cls.__name__=='Content':
return Column('id', Integer, primary_key=True)
else:
return Column('id', Integer, ForeignKey('content.id'),
primary_key=True)
class Content(HasId, Base):
content_type = Column(String(20))
@declared_attr
def __tablename__(cls):
return cls.__name__.lower()
class Project(Content):
currency = Column(String(3))
target = Column(Integer)
current = Column(Integer)
so lets tell declarative to pull off of HasId.id when it scans “Project”,
instead of getattr(Project, “id”):
--- a/lib/sqlalchemy/ext/declarative/base.py
+++ b/lib/sqlalchemy/ext/declarative/base.py
@@ -126,7 +126,7 @@ def _as_declarative(cls, classname, dict_):
"on declarative mixin classes.")
elif isinstance(obj, declarative_props):
dict_[name] = ret = \
- column_copies[obj] = getattr(cls, name)
+ column_copies[obj] = getattr(base, name)
if isinstance(ret, (Column, MapperProperty)) and \
ret.doc is None:
ret.doc = obj.__doc__
your test case works, but lets see the tests - six errors and four failures:
https://gist.github.com/zzzeek/8925277
So we can’t do that. You know I love the mixins/declared_attr thing, but
it’s hard. There’s craploads of tests for it and the amount of decisions we’ve
packed in there is already pretty intricate.
There’s some other variants that reduce the number of test failures, like this:
diff --git a/lib/sqlalchemy/ext/declarative/base.py
b/lib/sqlalchemy/ext/declarative/base.py
index 4fda9c7..b74809e 100644
--- a/lib/sqlalchemy/ext/declarative/base.py
+++ b/lib/sqlalchemy/ext/declarative/base.py
@@ -126,7 +126,7 @@ def _as_declarative(cls, classname, dict_):
"on declarative mixin classes.")
elif isinstance(obj, declarative_props):
dict_[name] = ret = \
- column_copies[obj] = getattr(cls, name)
+ column_copies[obj] = obj.__get__(obj, cls)
if isinstance(ret, (Column, MapperProperty)) and \
ret.doc is None:
ret.doc = obj.__doc__
but still lots of failures; there’s a lot of use cases that just expect
@declared_attr to hit once, then replace itself on the mapped class.
So the next idea is to just make a whole different command to do this, I
sketched that up here: www.sqlalchemy.org/trac/ticket/2952
but I’m not really sure what I want to do with that. mixins work great for
single classes but within a hierarchy not so much, and adding more options
makes me worried that we’re just adding flags rather than fixing the central
idea somehow.
signature.asc
Description: Message signed with OpenPGP using GPGMail
