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

tweak explanation of numeric "snapshots"; add abs/0 #2676

Closed
wants to merge 2 commits into from

Conversation

pkoppstein
Copy link
Contributor

Clarify and expand explanation of the snapshots that are taken of some numbers.

abs/0 is defined without reference to type.

jq.test now includes tests for abs and fabs, and includes tests for length@number

Clarify and expand explanation of the snapshots that are taken of some numbers.

abs/0 is defined without reference to type.

jq.test now includes tests for abs and fabs, and includes tests for length@number

prints as:

1.000000000000000000000000000000000000000000000000000002
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should say "1.000000000000000000000000000000000000000000000000000002"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tx. Done.


examples:
- program: 'map(abs)'
input: [-10, -1.1, -1e-1, 1000000000000000002, -1000000000000000002]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value needs to be quoted.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops. Done.

`tostring` uses the snapshot representation, if available, as
illustrated by these examples:

"-1.000000000000000000000000000000000000000000000000000002" | tonumber
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add more 4 spaces indentation to make them look like code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tx. Done.

# abs, fabs, length
abs
"abc"
"abc"
Copy link
Contributor

@itchyny itchyny Jul 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How should it work against null and booleans? Emits error in the current implementation but should be as it is?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And how should it work with strings, arrays, and objects as inputs? Currently it passes them through. That's very strange, but it's an artifact of strings, arrays, and objects sorting after numbers in jq, so this abs/0 doesn't want to negate them, so there's no error, but null, false, and true sort ahead of numbers, so abs/0 does want to negate them, and that then raises an error.

We have some options

  • leave as-is
  • the abs of all non-number values should be the identity
  • the abs of all non-number values should be an error

@pkoppstein
Copy link
Contributor Author

As @nicowilliams wrote:

We have some options

  • leave as-is
  • the abs of all non-number values should be the identity
  • the abs of all non-number values should be an error

In case it's not obvious, the main reason I went with a simple implementation is speed, but the other reason is that jq often eschews type-checking in favor of having a simple definition that can serve both as "specification" and "implementation". Since manual.yml makes that point by including a copy of the def, I did the same for abs.

And speaking of map, although the analogy is obviously not perfect, the fact is that map could have been written to issue a map-specific error if its input would raise an error, but it wasn't, even though one can easily adduce arguments to the contrary.

Admittedly, in this case, these considerations (of speed, brevity, and my interpretation of "the jq way") are not so weighty, so if the current implementation is deemed unacceptable, I'd go with insisting on type == "number", since that is probably expected, and since it allows future constraint relaxation.

@pkoppstein
Copy link
Contributor Author

This PR is superseded by #2767

@pkoppstein pkoppstein closed this Jul 25, 2023
@pkoppstein pkoppstein deleted the abs branch July 25, 2023 07:15
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