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

NaNs are weird (dup of #750) #814

Closed
blkqi opened this issue Jun 17, 2015 · 8 comments
Closed

NaNs are weird (dup of #750) #814

blkqi opened this issue Jun 17, 2015 · 8 comments

Comments

@blkqi
Copy link

blkqi commented Jun 17, 2015

I am getting strange results using the add function.

Reproduce by grabbing my sample data:

% curl http://sprunge.us/TfZU > test.json

The following does not give the result I expect:

% cat test.json | jq -s 'map(.a / .b) | add'
null

Yet this does:

% cat test.json | jq -s 'map(.a / .b)' | jq 'add'
515.764540663807
@nicowilliams
Copy link
Contributor

This is a result of NaNs getting displayed as null, but actually not being null.

So when .b is zero you get a NaN, which on output is shown as null, but if you try to add a NaN and something else you get another NaN, so as soon as one .b in your input is zero your sum gets poisoned.

Piping the output of jq to jq maps all those NaNs to nulls (and add ignores nulls).

You need to filter out cases where .b is zero.

More generally I think it'd be nice if we has an isnan function, and/or perhaps / ought to raise an error rather than output a NaN.

This works:

% jq -s 'map(if .b != 0 then .a / .b else empty end)|add' test.json
515.764540663807

Comments?

@nicowilliams
Copy link
Contributor

See also #750.

Options:

a) add an isnan builtin
b) make type output "null" when . is a NaN
c) make division/remainder operators raise an error when the divisor is zero
d) (c) and one or both of (a) and (b)
e) check for NaNs inputs in all arithmetic operators

I'm disinclined to implement (e). (a) seems harmless. I'm not sure about (b). (c) seems fine.

Also, I suppose we could go with (a) and (c) and add a builtin to produce a NaN just in case one really wants map(try (.a / .b) catch nan) | add.

EDIT: Fix syntax.

@nicowilliams nicowilliams changed the title Inconsistency in add function NaNs are weird (dup of #750) Jun 18, 2015
@nicowilliams
Copy link
Contributor

Making division/remainder operators raise an error is tricky: because of constant folding a constant expression like 1/0 is "run" at compile-time, so we'd have to catch that in the parser.

@nicowilliams
Copy link
Contributor

You can now (in master) use jq 'map((.a / .b)?) | add'.

EDIT: Indeed, you'll have to!

@blkqi
Copy link
Author

blkqi commented Jun 18, 2015

thanks @nicowilliams for the explanation and the suggestion

I tried filtering out the zero division case before filing this issue, but I must have fouled that up. Your example works.

@pkoppstein
Copy link
Contributor

@nicowilliams - Progress to be sure, but in a way, we're back to Square 1:

$ jq --version
jq-1.5rc1-105-gb9c2a32

$ jq -n ' inf/inf'
null

$ jq -n ' (inf/inf) + 0'
null

$ jq -n 'null + 0'
0

We'd probably need to use the strings "Infinity" and "NaN" in a consistent way to make these anomalies go away.

@nicowilliams
Copy link
Contributor

On Wed, Jun 17, 2015 at 07:46:26PM -0700, pkoppstein wrote:

@nicowilliams - Progress to be sure, but in a way, we're back to Square 1:

$ jq --version
jq-1.5rc1-105-gb9c2a32

$ jq -n ' inf/inf'
null

It's a NaN. It gets printed as null because JSON has no way to represent NaNs.

Division will raise when dividing by zero, and only then.

We'd probably need to use the strings "Infinity" and "NaN" in a consistent way to make these anomalies go away.

jq simply can't do that; see above. But users can now replace NaNs with "NaN" if they like, with ... | if isnan then "NaN" else . end | ....

@nicowilliams
Copy link
Contributor

Nor can jq raise an error when outputting a value that contains a NaN: the NaN might be too deeply buried in the value. Since jq has been outputting NaNs as null, that is what it should continue to do. The only other option is to have every arithmetic operation (and every libm function) raise an error when it outputs an infinity or a NaN -- maybe worth doing, but I'm not sold, especially with predicates like isnan around: I'd rather let the user do math and map NaNs to whatever they like.

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

No branches or pull requests

3 participants