On Jan 15, 2013, at 2:58 PM, Luca Wehrstedt wrote: > Thanks for the reply. > > I started diving into the code to debug the issues and, perhaps, write a > patch to fix them and I think I discovered their causes. > > The problem with the "link" decorator is the following: > the "@collection.link"[1] decorator sets the "_sa_instrument_role" of the > method to "link"; > the "_instrument_class" function correctly recognizes this role and, after > doing all its magic, sets the "_sa_link" attribute of the class to be the > linker method; > the "link_to_self"[3] and "unlink"[4] methods of "CollectionAdapter" check > the "_sa_on_link" attribute for existence and, in case, call it. > As you can see the issue is a name mismatch, therefore the solution is to > change all names to "_sa_link", to "_sa_on_link" or, my favorite, > "_sa_linker", for consistency with the other roles (appender, remover, > iterator, converter). > > Can you fix it right away or do I have to open a bug and provide a patch on > the bug tracker?
an expedient way to work here is to provide pull requests at https://bitbucket.org/sqlalchemy/sqlalchemy . > > The problem with the converter is more subtle. It's caused by the fact that I > am deriving my class from MappedCollection, which gets instrumented before my > class. Now that is kind of interesting, because at some point I found a bug related to the fact that MappedCollection was not being instrumented soon enough, so I added some explicit module-level _instrument_class() calls at the bottom of the module (you can see that was ticket 2406). I was unsure if there was some reason for that deferred instrumentation, and perhaps this was it. But it's not something that should be relied upon in any case. > Thus, after instrumentation, MappedCollection has two methods marked with > "_sa_instrument_role" of "converter": the "_convert" method defined by > MappedCollection itself and the "_sa_converter" method added by > "_instrument_class". When I define my class I override at most one of these > (or possibly none, if I call my converter, for example, "convert"). This > causes my class to have many different methods marked as converters and, for > some strange reasons, SA seems to always choose the ones I inherit from > MappedCollection (and not the one that I define, that never gets called). Yeah these are not mechanics I'm intimately familiar with. But there should be a way to fix this. > > Once I discovered the problem the workaround easily followed: I define my > converter, called "convert", and then I set "_convert = convert" and > "_sa_converter = convert". Sure, this is not an optimal solution, but it only > happens when one derives from MappedCollection (not when one creates a new > collection from scratch) and fixing it properly would require large changes > to the collection instrumentation mechanism and therefore I think it's an > acceptable solution. It may be worth to say something about it in the > documentation. I'd rather find a way to make it work more naturally without workarounds. One advantage to the fact that the methods aren't working, is that we can be sure nobody's using them. So if we need to change the API a bit, that's not a terrible thing. There probably should be a ticket for all of this regardless of pull requests so feel free to open something up on the tracker. -- 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 sqlalchemy+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/sqlalchemy?hl=en.
