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

std.sort does not accept second parameter #36

Closed
javaduke opened this issue Oct 1, 2019 · 6 comments
Closed

std.sort does not accept second parameter #36

javaduke opened this issue Oct 1, 2019 · 6 comments

Comments

@javaduke
Copy link
Contributor

javaduke commented Oct 1, 2019

When trying to sort an array of objects using std.sort with second parameter, e.g.
std.sort(myArray, function(x) x.weight) I'm getting this error:

sjsonnet.Error: Too many args, function has 1 parameter(s)

@javaduke
Copy link
Contributor Author

javaduke commented Oct 4, 2019

and looks like there are many more functions that don't take key function parameter.

@yehaolan
Copy link

yehaolan commented Oct 7, 2019

Same with std.set(...)

My jsonnet file has following code:

local a = {
  name: "123",
  id: "abc",
};

local b = {
  name: "123",
  id: "aaa",
};

local l = [a, b];

std.set(l, function (o) std.toString(o))

Then, I will get sjsonnet.Error: Too many args, function has 1 parameter(s)

@lihaoyi-databricks
Copy link
Contributor

This appears to be a new feature in jsonnet, with many of the std.* functions now taking an additional keyF argument with a default value of id.

@sparkprime @sbarzowski do you guys have any test suite for calls to these functions with the additional keyF parameter? I didn't see any such calls in the https://github.com/google/jsonnet/blob/master/test_suite/stdlib.jsonnet test files, but if you guys have a test suite I'd rather re-use yours rather than trying to come up with my own

@sbarzowski
Copy link
Contributor

Hmmm... it appears in the benchmark, in the docs and tests specific to go-jsonnet, but it's missing from the main test suite. I have created an issue to add some: google/jsonnet#716

@javaduke
Copy link
Contributor Author

javaduke commented Oct 8, 2019

This appears to be a new feature in jsonnet, with many of the std.* functions now taking an additional keyF argument with a default value of id.

@lihaoyi-databricks I've implemented this functionality in my fork, if you want, feel free to take a look, and I can create a PR for you.
The functions I modified are:
sort()
uniq()
set()
setMember()
setUnion()
setInter()
setDiff()

See here: https://github.com/modusintegration/sjsonnet/blob/master/sjsonnet/src/sjsonnet/Std.scala#L487-L606
https://github.com/modusintegration/sjsonnet/blob/master/sjsonnet/test/src/sjsonnet/StdWithKeyFTests.scala

@lihaoyi-databricks
Copy link
Contributor

@javaduke yes I would love a pull request!

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

No branches or pull requests

4 participants