search

visualfabriq / bquery

A query and aggregation framework for Bcolz (W2013-01)
https://www.visualfabriq.com
BSD 3-Clause "New" or "Revised" License
56 stars 11 forks source link

Python 3 Fixes #40

Closed waylonflinn closed 9 years ago

waylonflinn commented 9 years ago

Enable use of bquery with Python 3, while maintaining compatibility with 2.7

FrancescElies commented 9 years ago

Hi, thanks for this PR, though I won't have time until next week to look at it, the only suggestion I would have at the moment would be to modify https://github.com/visualfabriq/bquery/blob/master/.travis.yml#L7 to tell Travis to execute tests for python3 desired version too

waylonflinn commented 9 years ago

@FrancescElies thanks for the suggestion. That makes it much easier for everyone to see which test is failing and why.

CarstVaartjes commented 9 years ago

@FrancescElies do you know why we're not outputting the results in a sorted order here?

FrancescElies commented 9 years ago

Hi guys sorry for my late answer,

the iterable passed to itertools.groupby has to be sorted, https://github.com/visualfabriq/bquery/blob/master/bquery/tests/test_ctable.py#L110-L111, that's why itertools always return results with alphabetic order. As waylonflinn noted in some tests you'll see bcolz results being sorted before comparing them to itertools result.

Sorting the test input forces bcolz and itertools to have the same sorted dataset, so that's a good workaround. I also realised that failing test wasn't very readable, and using dynamic datasets depending on random, maybe not the best idea, both things was not making things easier to debug I guess. Maybe if we had have a test with a smaller and dataset would have been easier (maybe something like https://github.com/FrancescElies/bquery/blob/master/bquery/tests/test_ctable.py#L671-L699). If you have any suggestions for future tests, please share

Thanks!