-
Notifications
You must be signed in to change notification settings - Fork 18
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
multipleOf running into floating point issue #43
Comments
Can you describe the error you get? Basically multipleOf with floats does work as expected. For example the following tests are successful: https://github.com/sagold/json-schema-library/blob/main/test/unit/issues/issue43.multipleOf.float.test.ts. In general, the number type of javascript does have issues for small decimals and division on decimals. So if you need a more robust solution to restrict decimal length, while still maintaining the number type, I recommend a custom keyword, for example maxDigits: 2. import { Draft07, JsonValidator } from "json-schema-library";
const jsonSchema = new Draft07(mySchema, {
validateKeyword: {
// register validator for maxDigits
maxDigits: maxDigitsValidator as JsonValidator
},
// register keyword to numbers (adding maxDigits to the list)
typeKeywords: {
number: draft07Config.typeKeywords.number.concat("maxDigits")
}
}); with the validator something like const maxDigitsValidator: JsonValidator = (draft, schema, value: number, pointer) => {
if (isNaN(value) || isNaN(schema.maxDigits)) {
return undefined;
}
const [_, digits] = `${value}`.split('.');
if (digits && digits.length > schema.maxDigits) {
return {
type: 'error',
code: 'max-digits-errors',
name: 'MaxDigitsError',
message: `number should not have mor than ${schema.maxDigits} digits`,
data: { pointer, value, schema }
}
}
} As for the error generatiion it is recommended to follow the custom error introduction. |
That's great. Thanks for the detailed list of options. I'll consider them! In our case it's tricky as we're using a non-JS backend and JS front-end and finding JSON Schema has a lot of peculiarities that make it tough to keep them working exactly the same.So custom stuff we're trying to avoid, because the implementation will likely be difficult in Kotlin. An example where the lib doesn't work as expected is
|
You can also run something like this and get so many random ones: for (let i = 1; i < 100; i++) { let num = `2.${i}`; console.log(num, draft.validate(parseFloat(num))); }
|
Yes, you can see this also in you browser console
floating point arithmetic is a known issue and, to my knowledge, unsolvable. For example: ajv-validator/ajv#84 (comment). To work around most issues json-schema-library (like other validators) shifts decimals by multiplying it with a constant, but in this case javascript results to the following
Changing the number to
results in a correct value, which you can configure in the settings
but I would not rely on it and use the regex-pattern approach you mentioned. This would align the validation between the programming languages. I am very open to other suggestions |
Update I replaced the multipleOfPrecision proposal with the following solution: export function getPrecision(value: number): number {
const string = `${value}`;
const index = string.indexOf(".");
return index === -1 ? 0 : string.length - (index + 1);
}
function validateMultipleOf(draft, schema, value, pointer) {
if (isNaN(schema.multipleOf) || typeof value !== "number") {
return undefined;
}
const valuePrecision = getPrecision(value);
const multiplePrecision = getPrecision(schema.multipleOf);
if (valuePrecision > multiplePrecision) {
// higher precision of value can never be multiple of lower precision
return draft.errors.multipleOfError({
multipleOf: schema.multipleOf,
value,
pointer,
schema
});
}
const precision = Math.pow(10, multiplePrecision);
const val = Math.round(value * precision);
const multiple = Math.round(schema.multipleOf * precision);
if ((val % multiple) / precision !== 0) {
return draft.errors.multipleOfError({
multipleOf: schema.multipleOf,
value,
pointer,
schema
});
}
return undefined;
}
new Draft07(
{
type: "number",
multipleOf: 0.01,
multipleOfPrecision: 2
},
{
validateKeyword: {
multipleOf: validateMultipleOf
}
}
); You should be able to add this configuration in your code or install with Cheers, |
This is so great. Thanks for your thorough explanation and showing us all of this magic, not to mention fixing the "unsolvable" :D |
From my colleague Jemma about your fix
🏆 |
😀 Yes, the unsolvable label was set too quickly. Thank you, you are welcome! So does this solve your issue? Then we could merge this. |
It does yes!
…On Tue, 15 Aug 2023 at 4:07 pm, Sascha Goldhofer ***@***.***> wrote:
😀 Yes, the *unsolvable* label was set too quickly. Thank you, you are
welcome!
So does this solve your issue? Then we could merge this.
—
Reply to this email directly, view it on GitHub
<#43 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAB4C76ADPHFF7MA6ZVLJ73XVOGBJANCNFSM6AAAAAA3Q3JZQM>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
This issue is fixed with Thank you for your feedback! |
We have the use case of wanting to enforce numbers having no more than 2 decimal places so we're using the
multipleOf
keyword with.01
to try to achieve this. Unfortunately it fails in unexpected ways.There is a well known issue with
multipleOf
keyword when used with floating point numbers:Do you think this is something that this parser would like to fix or should we just use strings with regexes to achieve a similar result?
The text was updated successfully, but these errors were encountered: