coerce is being called on the base not the derived class

[Bert JW Regeer]

I am using a MutableComposite class, which has a classmethod called coerce(), the idea being that I can force coercion from one type to my custom type that does the right thing in the table.

This unfortunately does not work with MutableComposite as the coerce seems to only get called on the base and never on my custom derived class that contains the coerce() function.

I have attached a test case that showcases this, if I am using the system wrong, feel free to let me know.

[Bert JW Regeer]

Testcase that showcases the issue.

[Mike Bayer]

OK, the idea of "Composite" hadn't included built-in coercion before, and this is something you can get if you use a simple @validates decorator (feel free to use that for now), but yes if we just set the event with the per-property class and not the base then this is easily addable.

diff -r 4950b85e8384869d3f03498c6914afe5aadbf561 lib/sqlalchemy/ext/mutable.py
--- a/lib/sqlalchemy/ext/mutable.py Sun Dec 02 12:37:52 2012 -0500
+++ b/lib/sqlalchemy/ext/mutable.py Mon Dec 03 12:08:18 2012 -0500
@@ -572,7 +572,7 @@
             for prop in mapper.iterate_properties:
                 if (hasattr(prop, 'composite_class') and
                     issubclass(prop.composite_class, cls)):
-                    cls._listen_on_attribute(
+                    prop.composite_class._listen_on_attribute(
                         getattr(class_, prop.key), False, class_)

         event.listen(mapper, 'mapper_configured', listen_for_type)

needs docs + tests, backport to 0.7 as well

[Bert JW Regeer]

What can I do to help here? I am not really all that familiar with the SQLAlchemy code base and what is required to write docs/tests and or backport it.

[Mike Bayer]

actually it appears the issue is really more of how the event is getting assigned, and I might want to work some more with this. The _setup_listeners() method should only be established for the base just once with the above patch. otherwise this patch also works:

diff -r 4950b85e8384869d3f03498c6914afe5aadbf561 lib/sqlalchemy/ext/mutable.py
--- a/lib/sqlalchemy/ext/mutable.py Sun Dec 02 12:37:52 2012 -0500
+++ b/lib/sqlalchemy/ext/mutable.py Mon Dec 03 12:59:52 2012 -0500
@@ -567,7 +567,8 @@
         automatically.

         """
-
+        if cls.__name__ == 'MutableComposite':
+            return
         def listen_for_type(mapper, class_):
             for prop in mapper.iterate_properties:
                 if (hasattr(prop, 'composite_class') and
@@ -577,7 +578,6 @@

         event.listen(mapper, 'mapper_configured', listen_for_type)

-
 class MutableDict(Mutable, dict):
     """A dictionary type that implements :class:`.Mutable`.

[Mike Bayer]

Replying to xistence:

What can I do to help here? I am not really all that familiar with the SQLAlchemy code base and what is required to write docs/tests and or backport it.

Me or one of SQLA devs can do this ticket fairly quickly, though I want to play with the metaclass mechanics here (shouldn't really need a metaclass actually).

An immediate workaround is to do your date coercion in a @validates decorator, see http://docs.sqlalchemy.org/en/rel_0_8/orm/mapper_config.html?highlight=validates#simple-validators .

[Bert JW Regeer]

I have implemented the work-around for now, but as the person bringing the problem I always enjoy finding out how in the future I could help with patching code, and or solving the problem in a more direct manner. As in, I've got this problem, but I also have a possible solution. That and I enjoy fixing issues in open source that have a direct effect on what I do!

Thanks for the reply. Looking forward to seeing this fixed in an upcoming release :-).

[Mike Bayer]

well I had to think a bit how to do the event mechanics for this one, in conjunction with how the test suite runs (it clears out all events after each test), so the hackery on this one was pretty much on my end. The original patch is what I'd expect as a simple fix but I also wanted to end the practice of placing an event handler per mutablecomposite type, instead just using one event handler for all mutablecomposites. you'll see it in the patch.

for tests and docs, we have a way of doing that which if you just observe for awhile can be picked up.

[Mike Bayer]

d89d71d1dad4b795e14d5395f4008c5027b59baa 0.8

84e000f0766b8e4f9e2cd48615720c32c234ffa0 0.7

[Bert JW Regeer]

My work-around that I had implemented was using a _pubdate composite name in the class, and used hybrid_methods to do the setting/getting as required however I figured I'd simplify and use the @validates method, and it doesn't seem to be working:

    @validates('pubdate')
    def validate_pubdate(self, key, value):
        print "Validating pubdate"
        if not isinstance(value, Date):
            if isinstance(value, datetime.datetime):
                return Date(value.year, value.month, value.day)

            raise ValueError
        else:
            return value

Adding that to TestModel in my testcase does not actually call that function, it still calls coerce directly on the base. Now I understand that with the changes referenced above it doesn't make a difference in that a coerce on the derived class of a MutableComposite will take care of it they are currently not equivalents.

Will open a new ticket if required!

---

This works (probably as expected):

    _pubdate = composite(Date, 'year', 'month', 'day')

    @hybrid_property
    def pubdate(self):
        return self._pubdate

    @pubdate.setter
    def pubdate_set(self, value):
        print "Validating pubdate"
        if not isinstance(value, Date):
            if isinstance(value, datetime.datetime):
                self._pubdate = Date(value.year, value.month, value.day)
            else:
                raise ValueError
        else:
            self._pubdate = value

[Mike Bayer]

Removing milestone: 0.8.0final (automated comment)