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

docs: add note clarifying multipleOf implementation #1206

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

inclinedadarsh
Copy link

What kind of change does this PR introduce?

Documentation update

Issue Number:

Screenshots/videos:

image

Summary

Add a note explaining the difference between float-point implementation of programming languages and json schema

Does this PR introduce a breaking change?

No, it does not.

Other

I believe this note can be clearer, better if I also provide an example, not sure about it though. Please let me know about it.

add a note explaining the difference between float-point implementation of programming languages and json schema
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome to the JSON Schema Community. Thanks a lot for creating your first pull request!! 🎉🎉 We are so excited you are here! We hope this is only the first of many! For more details check out README.md file.

Copy link

github-actions bot commented Dec 23, 2024

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name Status Preview Last Commit
website ✅ Ready (View Log) Visit Preview 87d9f72

Copy link

codecov bot commented Dec 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (8676fc2) to head (87d9f72).
Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main     #1206   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           10        10           
  Lines          373       373           
  Branches        94        94           
=========================================
  Hits           373       373           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@jdesrosiers jdesrosiers left a comment

Choose a reason for hiding this comment

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

Thanks for giving this a start. There are a few things that need a second draft.

The JSON Specification allows an arbitrary implementation of the multipleOf keyword

The JSON Schema Specification defines multipleOf, not the JSON Specification. There is nothing arbitrary about multipleOf. It has a precise definition and consistent behavior across implementations. I'm not sure what this statement is trying to communicate to the reader, but it's full of inaccuracies.

and is not constrained by the floating-point behavior of most programming languages.

This part is correct.

In contrast, most programming languages adhere to the IEEE754 standard for floating-point numbers, which can result in discrepancies between validation according to the JSON Specification and the behavior in programming languages.

This is correct, but not the way I would frame it. I would say that with JSON Schema you don't need to worry about the limitations of IEEE754 floating-point numbers like you do in most programming languages.

@inclinedadarsh
Copy link
Author

Thanks a lot for the review @jdesrosiers

Thank you specifying exact changes needed, I'll be fixing these and would push the changes as soon as possible.

Copy link
Member

@jdesrosiers jdesrosiers left a comment

Choose a reason for hiding this comment

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

This sounds better but there's still an inaccuracy.

@@ -162,6 +162,7 @@ The multiple can be a floating point number:
4.021
```

<Infobox label="Note"> The JSON Schema Specification defines numerical precision independently of the IEEE 754 standard. This means developers do not need to worry about the typical limitations of floating-point arithmetic, such as precision loss or representation errors, which are common in most programming languages. </Infobox>
Copy link
Member

Choose a reason for hiding this comment

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

You still have this a little confused. The JSON specification defines numbers. The JSON Schema specification defines multipleOf using JSON numbers.

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.

[📝 Docs]: Erroneous example for multipleOf
2 participants