On Jun 4, 2010, at 3:21 PM, Gunnlaugur Briem wrote: > Hi, > > if I declaratively map a class with a mix-in and with a __table__ > definition that contains the columns in the mix-in, like so: > > class MyMixin(object): > id = Column(Integer, primary_key=True) > def foo(self): > return 'bar'+str(self.id) > > class MyModel(Base,MyMixin): > __table__ = Table('test', Base.metadata, > Column('id', Integer, primary_key=True), > Column('name', String(1000), nullable=False, index=True) > ) > > … then I get a fairly cryptic error: > > Traceback (most recent call last): > File "mixin_trouble.py", line 12, in <module> > class MyModel(Base,MyMixin): > File "/Users/gthb/extsvn/sqlalchemy/lib/sqlalchemy/ext/ > declarative.py", line 830, in __init__ > _as_declarative(cls, classname, cls.__dict__) > File "/Users/gthb/extsvn/sqlalchemy/lib/sqlalchemy/ext/ > declarative.py", line 681, in _as_declarative > if tablename or k not in parent_columns: > TypeError: Error when calling the metaclass bases > argument of type 'NoneType' is not iterable > > The real reason is that the case of __table__ with a mix-in is not > handled. If MyModel were defined with __tablename__ and id = > Column(Integer, primary_key=True), then tablename would be truey and > parent_columns would not be checked, so this would not come up. > > We could just improve the error message by checking for this > unsupported case. But it seems fairly easy to support it. This simple > change makes the above case work, and breaks no unit tests: > > diff -r dd463bfb847d lib/sqlalchemy/ext/declarative.py > --- a/lib/sqlalchemy/ext/declarative.py Mon May 31 15:22:55 2010 -0400 > +++ b/lib/sqlalchemy/ext/declarative.py Fri Jun 04 19:05:16 2010 +0000 > @@ -670,7 +670,9 @@ > "Columns with foreign keys to other > columns " > "are not allowed on declarative > mixins at this time." > ) > - if name not in dict_: > + if name not in dict_ and not ( > + '__table__' in dict_ and name in > dict_['__table__'].c > + ): > > potential_columns[name]=column_copies[obj]=obj.copy() > elif isinstance(obj, RelationshipProperty): > raise exceptions.InvalidRequestError( > > I may be missing something else that needs to be done in this case, > but in any case this gets my codebase (which I've just modified to use > a mix-in like this instead of a metaclass) running and passing its > tests without problems.
as long as all tests pass it is fine. If you could give me a patch that includes a test for this in test_declarative, that would be supremely helpful (if you want to make a trac ticket and target it at the 0.6.2 milestone). > > Also, since I've got your attention there, shouldn't this place really > check (in both this new __table__ case and the already-supported > __tablename__ case) that the column definitions are identical, or at > least mostly equivalent? If they don't, then the mix-in class' > expectations are not fulfilled and that may cause all sorts of trouble > so it's better to bump into it right away. do you mean, if the new class overrides what the mixin provides ? i'm not sure why we'd need to do anything if the class overrides the mixin. > > To that end, is there an existing way to check column definition > equality/equivalence, i.e. comparing at least type, constraints and > default and primary_key, and maybe also autoincrement, base_columns, > etc.? theres some of that in the unit tests but in general I try not to make such functionality a requirement of anything, as it's difficult to maintain and also has unresolved edge cases in the case of custom subclasses, custom types, dialect-specific types, potential dialect-specific flags, etc. -- You received this message because you are subscribed to the Google Groups "sqlalchemy" group. To post to this group, send email to sqlalch...@googlegroups.com. To unsubscribe from this group, send email to sqlalchemy+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/sqlalchemy?hl=en.