returning None (or anything) from @declared_attr means it won't be called further, though this usage may be intuitive on AbstractConcreteBase
this is a conceptual one that might not be possible to fix. With AbstractConcreteBase, we're both "abstract" and "mapped", so the scheme below is intuitive. But because it returns None, the logic in extract_mappable_attributes ("# using @declared_attr for some object that isn't Column/MapperProperty; remove from the dict and place the evaluated value onto the class.") blows it away.
need to re-identify the rationale for the "remove the attribute" logic, determine if the "might return None" style is likely only with AbstractConcreteBase or in a more general sense, then identify a potential warning, or even behavioral change, to make this more intuitive. A special symbol like "don't map" might serve as a more explicit replacement for None.
from sqlalchemy import *
from sqlalchemy.orm import *
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.ext.declarative import declared_attr
from sqlalchemy.ext.declarative import AbstractConcreteBase
Base = declarative_base()
class FooBase(AbstractConcreteBase, Base):
@declared_attr
def foo(cls):
print "FOO! ", cls.__name__
if cls.__name__ == 'A':
#return Column(String) # gets discarded, but affects B
return None # causes attr to not be called again
else:
return Column(Integer)
class A(FooBase):
__tablename__ = 'a'
id = Column(Integer, primary_key=True)
__mapper_args__ = {
'polymorphic_identity': 'a'
}
class B(FooBase):
__tablename__ = 'b'
id = Column(Integer, primary_key=True)
__mapper_args__ = {
'polymorphic_identity': 'b'
}
configure_mappers()
print FooBase.__mapper__.local_table
e = create_engine("sqlite://", echo=True)
Base.metadata.create_all(e)
output:
SELECT b.id, 'b' AS type
FROM b UNION ALL SELECT a.id, 'a' AS type
FROM a
CREATE TABLE a (
id INTEGER NOT NULL,
PRIMARY KEY (id)
)
CREATE TABLE b (
id INTEGER NOT NULL,
PRIMARY KEY (id)
)
bug is "foo" is missing from "b", "foo" missing from polymorphic selectable.
Comments (8)
-
reporter -
reporter - changed milestone to 1.2
-
reporter https://gerrit.sqlalchemy.org/290 - add a new constant, warn for ambiguous values
-
reporter - edited description
-
reporter - edited description
-
reporter the "setattr" logic just needs to not take place for declared attrs:
diff --git a/lib/sqlalchemy/ext/declarative/base.py b/lib/sqlalchemy/ext/declarative/base.py index 95f02ea..653c57c 100644 --- a/lib/sqlalchemy/ext/declarative/base.py +++ b/lib/sqlalchemy/ext/declarative/base.py @@ -278,8 +278,10 @@ class _MapperConfig(object): if k in ('__table__', '__tablename__', '__mapper_args__'): continue + FLAG = False value = dict_[k] if isinstance(value, declarative_props): + FLAG = True value = getattr(cls, k) elif isinstance(value, QueryableAttribute) and \ @@ -302,7 +304,8 @@ class _MapperConfig(object): # and place the evaluated value onto the class. if not k.startswith('__'): dict_.pop(k) - setattr(cls, k, value) + if not FLAG: + setattr(cls, k, value) continue # we expect to see the name 'metadata' in some valid cases; # however at this point we see it's assigned to something trying
-
reporter nope
def test_arbitrary_attrs_three(self): class Mapped(Base): __tablename__ = 't' id = Column(Integer, primary_key=True) @declared_attr def some_attr(cls): return cls.__name__ + "SOME ATTR" eq_(Mapped.some_attr, "MappedSOME ATTR") eq_(Mapped.__dict__['some_attr'], "MappedSOME ATTR")
-
reporter - changed status to resolved
Don't hard-evaluate non-ORM @declared_attr for AbstractConcreteBase
Fixed bug where using :class:
.declared_attr
on an :class:.AbstractConcreteBase
where a particular return value were some non-mapped symbol, includingNone
, would cause the attribute to hard-evaluate just once and store the value to the object dictionary, not allowing it to invoke for subclasses. This behavior is normal when :class:.declared_attr
is on a mapped class, and does not occur on a mixin or abstract class. Since :class:.AbstractConcreteBase
is both "abstract" and actually "mapped", a special exception case is made here so that the "abstract" behavior takes precedence for :class:.declared_attr
.Change-Id: I6160ebb3a52c441d6a4b663c8c9bbac6d37fa417 Fixes:
#3848→ <<cset 8b8236934764>>
- Log in to comment
will put this up in a gerrit for 1.2