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

Improve documentation for -precision CLI option #76

Closed
RespiteSage opened this issue Jun 5, 2024 · 1 comment
Closed

Improve documentation for -precision CLI option #76

RespiteSage opened this issue Jun 5, 2024 · 1 comment

Comments

@RespiteSage
Copy link

The Problem (in my opinion)

As of v1.8.1, the documentation for the -precision CLI option reads

(for jd without options or arguments)

  -precision=N Precision for numbers. Positive number for decimal places or
               negative for significant figures.

(for jd --help)

  -precision float
    	Precision for numbers

Precision for numbers doesn't provide enough information, and Positive number for decimal places or negative for significant figures is misleading and appears to be incorrect.

I misunderstood the documentation as saying that I was supposed to provide an integer number of decimal places or significant figures (in my case, something like jd -precision 12 one.json two.json). This obviously didn't do what I wanted, and all number values diffed as equal; I cranked the precision value up to 30, thinking that would make all of the (unequal) numbers with 12-15 digits after the decimal place diff as unequal, but they still diffed as equal. To understand what was happening, I had to look at the code:

func (n1 jsonNumber) Equals(node JsonNode, metadata ...Metadata) bool {

	precision := getPrecision(metadata)

	n2, ok := node.(jsonNumber)
	if !ok {
		return false
	}
	return math.Abs(float64(n1)-float64(n2)) <= precision
}

(link to source)

Okay, so when I tried

jd -precision 12 one.json two.json

what I actually should have done was

jd -precision 0.000000000001 one.json two.json

because the precision value is what I normally think of as a "delta" or "epsilon" value. I think precision is probably better than using the names of Greek letters (even if I'm personally used to doing that), but the word "precision" on its own doesn't obviously indicate what kind of value is expected in this context.

As for Positive number for decimal places or negative for significant figures, this is incorrect as far as I can tell. I'm not familiar with Go, but I couldn't see anywhere that the value provided by the user is modified before the Equals function above, which would mean that a negative precision value actually results in Equals always returning false, regardless of the absolute magnitude of the precision value.

Suggested Solutions

All I'm actually asking for is a clarification in the CLI option documentation. I just wanted to sufficiently motivate a change (though perhaps I overdid it). So here's example of what I'd like:

(for jd without options or arguments)

  -precision=N Precision for numbers, i.e. the maximum absolute difference for
             which jd still considers two numbers "equal".

(for jd --help)

  -precision float
    	Precision for numbers in the form of a maximum absolute difference value,
        like 0.001

Even better if you also add an example using this option, like

jd -precision=0.000001 a.json b.json

I'd be happy to submit a little PR for this, but I wanted to see what you think first, especially if I missed the handling for negative values or something.

@josephburnett
Copy link
Owner

@RespiteSage you're totally right. This was all originally in #63 and I'm not sure why the mention of negative numbers.

I've taken your suggestion and shorted it a bit to "Maximum absolute difference for numbers to be equal". And in the usage provided an example. Here is the change: 1f854f4 and it will be included in the next release.

Thanks for pointing this out!

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

2 participants