Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CLOSED] Hybrid and normal properties break joins and possible query params #4

Closed
albireox opened this issue Nov 2, 2016 · 4 comments
Labels
bug a general bug or other breaking feature
Milestone

Comments

@albireox
Copy link
Member

albireox commented Nov 2, 2016

Issue by albireox
Thursday Jul 07, 2016 at 06:43 GMT
Originally opened as https://github.com/marvin-manga/marvin/issues/4


Reported by Brian:

Not sure if there is/was a related ticket for this already. The hybrid property plateifu is causing issues. I suspect this will be generic for all added properties.

On the instance side, plateifu is a string. On the class side, plateifu is the concat function. This means that when cube.plateifu is requested as a query return parameter, the param_to_form_lookup.mapToColumn method returns a concat function and not a ModelClass? attribute.

So when we create the join tables out of these parameters at line 276 in join_tables, it crashes but plateifu concat does not have the .class property.

AttributeError: Neither 'concat' object nor 'Comparator' object has an attribute 'class_'

For all of our hybrid properties, and perhaps normal added properties, on the ModelClasses?, how will we generate the ModelClass? to pass into the joins?

Assigning to you José, since it's related to how we are handling parameters in the _param_to_form_lookup, plus the query parameters, plus how we create the list of Join ModelClasses?, but really just opening this up for discussion.

Things will continue to work as long as we don't query on any of these parameters, and plateifu is a doozy of a parameter.

@albireox
Copy link
Member Author

albireox commented Nov 2, 2016

Comment by albireox
Thursday Jul 07, 2016 at 06:45 GMT


Here's an update on this. Currently when we build our list of ModelClasses? from input queryparams to add into the joins, we use the .class_ property on the SQLalchemy InstrumentedAttributes?. This returns the class the attribute belongs to. Hybrid properties do not have this attribute. Hybrid properties are whatever they are, e.g. plateifu is a concat function, the NSA colors are BinaryExpressions?. We need a way to access the class these properties are attached to.

The SQLalchemy guy said that the .class_ attribute will become available to hybrid properties with version 1.1, available sometime this year. Or we can try to use the hybrid.py code from 1.1 in our 1.0X version, and "it should work ok". The 1.1 is available here

https://bitbucket.org/zzzeek/sqlalchemy/issues/3653

We could manually add the .class_ attribute. I tried this and it did not work. I tried
setattr(datadb.Cube.plateifu, 'class_', datadb.Cube.mangaid.class_)

We could build a Custom Comparator which converts Hybrid Properties into InstrumentedAttributes?, and then we'd have access to the .class_ method. The documentation for that is here
http://docs.sqlalchemy.org/en/latest/orm/extensions/hybrid.html#building-custom-comparators

I'm in the middle of attempting to get this to work, like so

    @hybrid_property
    def plateifu(self):
        '''Returns parameter plate-ifu'''
        return '{0}-{1}'.format(self.plate, self.ifu.name)

    @plateifu.comparator
    def plateifu(cls):
        return ConcatComparator(Cube.plate, IFUDesign.name)

    # @plateifu.expression
    # def plateifu(cls):
    #     return func.concat(Cube.plate, '-', IFUDesign.name)

with

class ConcatComparator(Comparator):
    ''' '''
    def __init__(self, A, B):
        self.concatparam = func.concat(A, '-', B)

    def __clause_element__(self):
        return self.concatparam

    def __eq__(self):
        return self.__clause_element__() == self.concatparam

    def __str__(self):
        return self.concatparam

datadb.Cube.plateifu now has the .class_ attribute, but does not work in a query. I'm still learning about this and trying to sort it out.

Or...we can try to do something else entirely where we utilize our param_form_lookup dictionary.

@albireox albireox added this to the Marvin Beta milestone Nov 2, 2016
@albireox albireox added the bug a general bug or other breaking feature label Nov 2, 2016
@albireox
Copy link
Member Author

albireox commented Nov 2, 2016

Comment by albireox
Thursday Jul 07, 2016 at 07:00 GMT


@havok2063, is this still a problem?

@albireox
Copy link
Member Author

albireox commented Nov 2, 2016

Comment by havok2063
Thursday Jul 07, 2016 at 13:32 GMT


@albireox yes this is still a problem. Our options are basically the same as above. We either wait for SQLalchemy 1.1 to be released supposedly sometime this summer, we try to find an alternate way of determining the ModelClass for which a parameter belongs other than class_, or we try to build a custom Comparator. The last one I was not successful at but also did not spend a lot of time on.

@albireox
Copy link
Member Author

albireox commented Nov 2, 2016

Comment by havok2063
Wednesday Sep 21, 2016 at 21:35 GMT


Upgrading to SQLAlchemy 1.1 fixes this issue. The hybrid_properties all now have a class_ attribute and can be returned as parameters in the query results. Tested with cube.plateifu.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug a general bug or other breaking feature
Projects
None yet
Development

No branches or pull requests

1 participant