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?

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. 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).

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.

Luca


[1] 
https://bitbucket.org/sqlalchemy/sqlalchemy/src/1a0cfc426ba23842ddf0c6f14ec4b51de98cb7b7/lib/sqlalchemy/orm/collections.py?at=default#cl-422
[2] 
https://bitbucket.org/sqlalchemy/sqlalchemy/src/1a0cfc426ba23842ddf0c6f14ec4b51de98cb7b7/lib/sqlalchemy/orm/collections.py?at=default#cl-877
[3] 
https://bitbucket.org/sqlalchemy/sqlalchemy/src/1a0cfc426ba23842ddf0c6f14ec4b51de98cb7b7/lib/sqlalchemy/orm/collections.py?at=default#cl-609
[4] 
https://bitbucket.org/sqlalchemy/sqlalchemy/src/1a0cfc426ba23842ddf0c6f14ec4b51de98cb7b7/lib/sqlalchemy/orm/collections.py?at=default#cl-615

-- 
You received this message because you are subscribed to the Google Groups 
"sqlalchemy" group.
To view this discussion on the web visit 
https://groups.google.com/d/msg/sqlalchemy/-/pTA5FvGrebMJ.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/sqlalchemy?hl=en.

Reply via email to