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

Fix floats not getting converted to ints after rounding + more strict equality in tests #219

Closed
wants to merge 3 commits into from

Conversation

null-dev
Copy link

Currently jaq's handling of floats differs from the behavior specified in the docs:

❯ jaq --version; jaq -n '1.2 | [floor, round, ceil]'
jaq 1.6.0
[
  1.0,
  1.0,
  2.0
]

git bisect shows that this started happening after this commit: dfc6fc0

This PR fixes the behavior so floats are converted to ints again after rounding:

❯ cargo run -- -n '1.2 | [floor, round, ceil]'
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.03s
     Running `target/debug/jaq -n '1.2 | [floor, round, ceil]'`
[
  1,
  1,
  2
]

I've also tried to tighten up the equality check used during tests so it fails:

  • If both values are of different types
  • Or if the fields in both values are in a different order

This was a little complicated, and I'm not sure if this is even something you want, if it's worth the extra complexity or if the implementation is to your liking. Feedback is welcome, or you can also just close this PR and cherry-pick the float fix yourself if you don't want the test equality changes.

@null-dev null-dev changed the title Fix floats not getting converted to ints after rounding + more strict Float fix Fix floats not getting converted to ints after rounding + more strict equality in tests Oct 21, 2024
@01mf02
Copy link
Owner

01mf02 commented Oct 23, 2024

Hi @null-dev, thanks a lot for your PR and for spotting this!

Indeed, I think that while your StrictEq approach has merits in tests, it does not correspond to an existing notion of equality in jq. Therefore, I have created my own commit that cherry-picks your rounding commit only. However, I'm now testing whether the output of round & friends is integer, by checking that [0, 1][1 | round] returns 1 (instead of an error, as it would have before your fix).

Thanks again!

@01mf02 01mf02 closed this Oct 23, 2024
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

Successfully merging this pull request may close these issues.

2 participants