-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Revisions to any/2, all/2, IN/1, IN/2 as per #1438 #1440
Conversation
…06 tests passed (0 malformed); no new valgrind issues
…. Also improve efficiency by eliminating array construction. To verify consumption is limited, use debug, e.g. limit(0; range(0;10) | debug)
./jqtest => 307 of 307 tests passed (0 malformed)
@@ -304,5 +301,8 @@ def JOIN($idx; stream; idx_expr): | |||
stream | [., $idx[idx_expr]]; | |||
def JOIN($idx; stream; idx_expr; join_expr): | |||
stream | [., $idx[idx_expr]] | join_expr; | |||
def IN(s): reduce (first(select(. == s)) | true) as $v (false; if . or $v then true else false end); | |||
def IN(src; s): reduce (src|IN(s)) as $v (false; if . or $v then true else false end); | |||
def IN(s): . as $in | first( if (s == $in) then true else empty end ) // false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. This can be further simplified as:
def IN(s): first(select(.==s), false);
def IN(src; s): first((src|IN(s)),false);
if . then break $out elif $i | condition then true else . end; | ||
if . then . else empty end)] | length == 1; | ||
|
||
def isempty(g): 0 == ((label $go | g | (1, break $go)) // 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isempty/1
is already in master. Can you rebase?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, never mind, you just moved it.
if . then . else empty end)] | length == 1; | ||
|
||
def isempty(g): 0 == ((label $go | g | (1, break $go)) // 0); | ||
def any(stream; predicate): isempty(stream | predicate or empty) | not; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! This is much simpler than the any/2
in master.
(true; | ||
if .|not then break $out elif $i | condition then . else false end; | ||
if .|not then . else empty end)] | length == 0; | ||
def all(stream; predicate):isempty(stream | predicate and empty); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice simplification!
def IN(src; s): any( src|IN(s); .); | ||
|
||
# f should always evaluate to a string: | ||
def GROUPS_BY(stream; f): reduce stream as $x ({}; .[$x|f] += [$x] ) | .[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be all lower-case. Though it's SQLish, it's also jq-ish.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nicowilliams - As I mentioned elsewhere, git and github have me all tied up in knots at the moment, so please make any changes you see fit.
I used uppercase for GROUPS_BY to emphasize the distinction with the existing group_by
. Incidentally, is it too late to re-implement group_by
so that it does not incur the costs of sorting? (For example, group_by
could be implemented using GROUPS_BY
.)
Here is what I believe now to be a much better GROUPS_BY:
It is better because it is efficient and versatile, while not being subject to all the caveats that come with the simpler GROUPS_BY. |
Revisions to any/2, all/2, IN/1, IN/2 as per #1438;
./jqtest 306 of 306 tests passed (0 malformed); no new valgrind issues