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

Enhance the "add" operator so that, when applied to objects, if two f… #1103

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cinquin
Copy link

@cinquin cinquin commented Feb 20, 2016

…ields with the same name have a numerical value the corresponding field of the result object is set to the sum of the field values. (The previous behavior was for one of the values to just be discarded.)

Before:

echo '[[{"a":1}, {"b":2}, {"a":3}]]' | jq "map(add)" 
[
  {
    "a": 3,
    "b": 2
  }
]

After:

echo '[[{"a":1}, {"b":2}, {"a":3}]]' | jq "map(add)"
[
  {
    "a": 4,
    "b": 2
  }
]

Perhaps this operator should be given a new name since users might be relying on its current behavior.

…ields with the same name have a numerical value the corresponding field of the result object is set to the sum of the field values. (The previous behavior was for one of the values to just be discarded.)
@cinquin
Copy link
Author

cinquin commented Feb 20, 2016

The test failure is due to a problem with the continuous integration system itself (as noted in other pull requests), and not to the changes in my pull request. The tests do pass on my machine.

@ghost
Copy link

ghost commented Feb 20, 2016

Note that this doesn't just change the behaviour of add (the builtin), but that of all addition operations between objects, such as +.

This is something that can and should be discussed, but I don't think add should add sub-elements recursively; and if it does, I don't think it should do it with numbers only. I'm not sure what the use case of this very specific tweak is (consider it only applies to numbers and not to strings, arrays or other objects, which also have + behaviours) and how it fits in the standard library. Similar functions in other languages, such as .extend in Javascript, do exhibit the behaviour previous to this pull request.

@cinquin
Copy link
Author

cinquin commented Feb 20, 2016

@slapresta My use case is combining objects that represent event counts. I want to derive a single object that contains aggregate counts (which I realize could probably be achieved using currently existing operators, in a way that I think would be more complicated).
In my mind it makes sense to combine the values in some way rather than to arbitrarily choose one value and discard the others.
I agree that it would make sense to extend this behavior to strings, etc.

@ghost
Copy link

ghost commented Feb 20, 2016

Well, choosing one of the two is what similar operators do: .extend() in JS, array_merge in PHP, .update() in Python...

Here's a way to solve your problem without modifying jq's builtins:

reduce .[] as $i ({}; ($i | to_entries[]) as $e | .[$e.key] += $e.value)

Over the input [{"a": 1}, {"b": 2}, {"a": 3}], this filter will output {"a": 4, "b": 2}.

@nicowilliams
Copy link
Contributor

Hi, first, I know the appveyor build is broken for reasons having nothing to do with this. I need to find the time to do something about it. (The Oniguruma pacman pkgs for msys broke.)

Second, I really appreciate this PR. I'm glad you're doing this. Thanks! However, this change would have backwards-compatibility repercussions that we can't really accept. Perhaps you could add a new jv function to do what you want and then export that function via src/builtin.c? That'd be great!

@cinquin
Copy link
Author

cinquin commented Feb 21, 2016

@nicowilliams Thanks for the feedback. (And thanks for jq!) I'm happy to update the patch to add a new builtin. Do you have a suggestion as to what name to use?

@nicowilliams
Copy link
Contributor

@cinquin You're welcome. The thanks for jq mostly go to @stedolan :)

I don't have a good suggestion for a good name for the new builtin. Maybe aggregate_add? Not sure.

@pkoppstein
Copy link
Contributor

An aggregate_add/0 can be defined quite adequately in jq itself, e.g.:

def aggregate_add:
  reduce .[] as $o ({};  reduce ($o|keys[]) as $key (.; .[$key] += $o[$key]));

However, the case for including such a filter (however named and with whatever tweaks to handle edge-cases) as a jq builtin seems to me to be quite weak, certainly in comparison with sigma/1 defined as follows:

def sigma(f): reduce .[] as $o (0; . + ($o | f )) ;

This of course corresponds with the widely understood and used Σ operation, but if the name "sigma" is deemed inappropriate here, the filter could be called summation.

For example, given an array of objects with names, heights, and weights, it is unlikely anyone will want to "add" the names, but using sigma/1, it is trivial to compute the sums of the heights and weights:

data | {height: sigma(.height), weight: sigma(.weight)}

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

Successfully merging this pull request may close these issues.

4 participants