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

Fixes error when using to group_by_* #115

Merged
merged 1 commit into from Mar 21, 2016
Merged

Fixes error when using to group_by_* #115

merged 1 commit into from Mar 21, 2016

Conversation

@leonelgalan
Copy link
Contributor

@leonelgalan leonelgalan commented Feb 22, 2016

Series was a range and needed to be converted to an array, this same conversion was done a couple of lines below. Closes #114

After posting the issue, I found out that the SQL was correct. Since this feature was added bd6b17d#diff-fa38e4a49c0b6d65a6d0a56acb87abf7R93 this line was missing the .to_a other lines had: bd6b17d#diff-fa38e4a49c0b6d65a6d0a56acb87abf7R107

Results for example posted in issue:

relation.group_by_hour_of_day(:created_at).group_by_day_of_week(:created_at).count
# => {[23, 0]=>186, [22, 0]=>337, [21, 0]=>539, [20, 0]=>602, [19, 0]=>1084, [18, 0]=>1270, [17, 0]=>1183, [16, 0]=>1337, [15, 0]=>1115, [14, 0]=>1376, [13, 0]=>1473, [12, 0]=>1495, [11, 0]=>776, [10, 0]=>393, [9, 0]=>293, [8, 0]=>214, [7, 0]=>78, [6, 0]=>55, [5, 0]=>49, [4, 0]=>23, [3, 0]=>47, [2, 0]=>134, [1, 0]=>151, [0, 0]=>341, [23, 1]=>226, [22, 1]=>300, [21, 1]=>505, [20, 1]=>803, [19, 1]=>1103, [18, 1]=>1584, [17, 1]=>1264, [16, 1]=>1028, [15, 1]=>1102, [14, 1]=>1340, [13, 1]=>1154, [12, 1]=>1130, [11, 1]=>840, [10, 1]=>669, [9, 1]=>326, [8, 1]=>244, [7, 1]=>236, [6, 1]=>93, [5, 1]=>83, [4, 1]=>38, [3, 1]=>15, [2, 1]=>8, [1, 1]=>40, [0, 1]=>148, [23, 2]=>205, [22, 2]=>403, [21, 2]=>934, [20, 2]=>1151, [19, 2]=>1373, [18, 2]=>1584, [17, 2]=>1373, [16, 2]=>1267, [15, 2]=>1306, [14, 2]=>1291, [13, 2]=>1087, [12, 2]=>1448, [11, 2]=>1155, [10, 2]=>842, [9, 2]=>361, [8, 2]=>289, [7, 2]=>239, [6, 2]=>94, [5, 2]=>141, [4, 2]=>21, [3, 2]=>8, [2, 2]=>5, [1, 2]=>65, [0, 2]=>111, [23, 3]=>258, [22, 3]=>379, [21, 3]=>817, [20, 3]=>1124, [19, 3]=>1337, [18, 3]=>1366, [17, 3]=>1161, [16, 3]=>1059, [15, 3]=>1343, [14, 3]=>1294, [13, 3]=>940, [12, 3]=>1472, [11, 3]=>1130, [10, 3]=>999, [9, 3]=>449, [8, 3]=>351, [7, 3]=>329, [6, 3]=>121, [5, 3]=>86, [4, 3]=>28, [3, 3]=>12, [2, 3]=>45, [1, 3]=>71, [0, 3]=>80, [23, 4]=>254, [22, 4]=>457, [21, 4]=>760, [20, 4]=>1122, [19, 4]=>1470, [18, 4]=>1702, [17, 4]=>1402, [16, 4]=>1192, [15, 4]=>1415, [14, 4]=>1444, [13, 4]=>1080, [12, 4]=>1368, [11, 4]=>1016, [10, 4]=>879, [9, 4]=>376, [8, 4]=>422, [7, 4]=>384, [6, 4]=>142, [5, 4]=>84, [4, 4]=>39, [3, 4]=>26, [2, 4]=>30, [1, 4]=>36, [0, 4]=>105, [23, 5]=>413, [22, 5]=>814, [21, 5]=>1449, [20, 5]=>1481, [19, 5]=>2049, [18, 5]=>2049, [17, 5]=>1669, [16, 5]=>1475, [15, 5]=>1308, [14, 5]=>1390, [13, 5]=>1292, [12, 5]=>1416, [11, 5]=>903, [10, 5]=>850, [9, 5]=>354, [8, 5]=>391, [7, 5]=>353, [6, 5]=>201, [5, 5]=>115, [4, 5]=>35, [3, 5]=>9, [2, 5]=>30, [1, 5]=>72, [0, 5]=>135, [23, 6]=>460, [22, 6]=>830, [21, 6]=>1090, [20, 6]=>1548, [19, 6]=>1856, [18, 6]=>2081, [17, 6]=>1725, [16, 6]=>1545, [15, 6]=>1369, [14, 6]=>1767, [13, 6]=>1738, [12, 6]=>1687, [11, 6]=>1126, [10, 6]=>644, [9, 6]=>435, [8, 6]=>352, [7, 6]=>177, [6, 6]=>75, [5, 6]=>70, [4, 6]=>36, [3, 6]=>8, [2, 6]=>108, [1, 6]=>140, [0, 6]=>283}
Series was a range and needed to be converted to an array, this same
conversion was done a couple of lines below. Closes #114
@ankane ankane merged commit 7347afc into ankane:master Mar 21, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
ankane added a commit that referenced this pull request Mar 21, 2016
@ankane
Copy link
Owner

@ankane ankane commented Mar 21, 2016

Great, thanks @leonelgalan! 👍 While investigating why this was calling reverse, I found another issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants