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

Additional equality metric accommodating JS numeric semantics? #156

Open
cemerick opened this issue Oct 25, 2022 · 3 comments · May be fixed by #186
Open

Additional equality metric accommodating JS numeric semantics? #156

cemerick opened this issue Oct 25, 2022 · 3 comments · May be fixed by #186

Comments

@cemerick
Copy link
Contributor

The equals implementation developed in #71 and #73 is a straightforward structural equality, and does good work in that context.

However, I occasionally find myself wishing there was an equality metric that accommodated JS-native semantics vis a vis numbers. The nit is that yojson always distinguishes between ints and floats, even if said floats have a zero fractional part, while JSON parsers in Javascript engines do not (e.g. JSON.parse("5.0")5). So, it's easy to produce yojson structures that are not equal in the strict structural sense as currently implemented in equals, but which are equal per JS semantics.

As things stand, the only generalized workaround is a rewrite of the yojson trees being compared with equal to truncate any zero-fractional `Float to its corresponding `Int.

Any interest in adding this?

@Leonidas-from-XIV
Copy link
Member

I very much understand the request, especially as JSON doesn't really have ints or floats, it just defines numbers. Structural equality is simple and more or less well-defined, but adding the JavaScript semantics of 5.0 == 5 raises the question of whether `List [] should be equal to `Int 0 be equal to `String "0". Further complicated by the fact that Yojson.Safe.t encodes arbitrary size numbers in `Intlit because of course the spec is vague what the range of numbers is.

If we find reasonable semantics for it, sure, I wouldn't mind including it, but I wouldn't want to add something half-baked that only works in some cases.

@cemerick
Copy link
Contributor Author

It's a fair question. I did aim to constrain the scope of things to (well-defined) numerics, for all the reasons you mentioned.

I personally don't think the truly degenerate cases of JS equality should be propagated anywhere ([] == 0 or 0 == "0" to use your examples). That feels like a very different category of problem than simple truncation of zero-fractional floats; an inherent footgun that might be nontrivial to get "right".

So, if the acceptable enhancement is "all of JS equality or nothing", I guess I'd say it's better to do nothing. 🤷

@Leonidas-from-XIV
Copy link
Member

Yes, if the only part is numeric equality, then it is ok. Likewise, I'm not too keen in implementing the whole JavaScript equality semantics (though js_equal would be a pretty descriptive name for that equality).

I'm slightly worried about the case of comparing `Float and `Floatlit (from Yojson.Raw.t), but I guess there are sane ways to implement that.

Leonidas-from-XIV added a commit to Leonidas-from-XIV/yojson that referenced this issue Jun 28, 2024
Leonidas-from-XIV added a commit to Leonidas-from-XIV/yojson that referenced this issue Jun 28, 2024
@Leonidas-from-XIV Leonidas-from-XIV linked a pull request Jun 28, 2024 that will close this issue
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 a pull request may close this issue.

2 participants