Pegase745 / sqlalchemy-datatables

SQLAlchemy integration of jQuery DataTables >= 1.10.x (Pyramid and Flask examples)
MIT License
158 stars 66 forks source link

yadcf_range_number causes TypeError: a float is required #120

Open louking opened 5 years ago

louking commented 5 years ago

I've been making a lot of progress using this fine package. Thanks again for your guidance earlier.

I'm trying to use yadcf_range_number and am seeing self.error = 'a float is required' in the output_result() call. Upon investigation, found the exception inside of _set_yadcf_data during the call to math.floor(v[0]), as the return from .add_columns() above returned (None, None). https://github.com/Pegase745/sqlalchemy-datatables/blob/049ab5f98f20ad37926fe86d5528da0c91cd462d/datatables/datatables.py#L79

Relevant code

model snippet

class Race(Base):
    distance = Column(Float)

(inside of view function)

            columns = [
                            : 
                ColumnDT(Race.distance, mData='miles', search_method='yadcf_range_number'),
                            : 
            ]
                            : 
            q = db.session.query().select_from(RaceResult).filter_by(**resultfilter).join(Runner).join(Race).join(Location)
            rowTable = DataTables(args, q, columns)
            output_result = rowTable.output_result()

Checked https://docs.sqlalchemy.org/en/13/orm/query.html#sqlalchemy.orm.query.Query.add_columns and don't see what add_columns() should be returning.

I do see a test for yadcf_range_slider for Integer in tests, but haven't tried to reproduce the testing environment for Float. But given where this is occurring doesn't seem like this should make a difference Integer vs. Float

Am I doing something wrong, or is this a bug?

image

louking commented 5 years ago

It dawned on me that the all_columns call is determining the min and max based on my query. If so this doesn't handle my use case. My initial query returns no rows unless one of the other filters is activated, therefore the (None, None) response.

In any case, I think the package should allow the caller to supply the yadcf_data_x values.

I'll do some more investigation if I can monkey patch something, or put in a PR with this capability.

louking commented 5 years ago

Looks like the easiest way is to create my own class based on DataTables and replace _set_yadcf_data.

I'd close this, but maybe you want to consider adding an optional yadcf_data parameter to ColumnDT to allow the caller to specify externally.