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

Enhancing best_match through by sorting the type of error #728

Closed
bliiben opened this issue Aug 23, 2020 · 3 comments · Fixed by #972
Closed

Enhancing best_match through by sorting the type of error #728

bliiben opened this issue Aug 23, 2020 · 3 comments · Fixed by #972
Labels
Enhancement Some new desired functionality

Comments

@bliiben
Copy link

bliiben commented Aug 23, 2020

This issue is a follow up on issue 698.

The feature request, describes how to enhance the best_match function in order to prioritize the type of the rule with the type of the instance. Example : if an array is presented in an instance, array related rules should be prioritized against other type of rules.

As shown in the following example, this currently isn't reliable :

The rule :

{
    "properties" : {
        "value" : {
            "anyOf" : [
                {"type" : "number","maximum":0},
                {"type" : "array", "items" : {"type":"number", "minimum" : 0}},
                {"type":"string", "const" : "check"}
            ]
        }
    }
}

With :

{"value" : 2}

Expected result (OK) :

2 is greater than the maximum of 0

Failed validating 'maximum' in schema[0]:
    {'maximum': 0, 'type': 'number'}

Current result :

2 is greater than the maximum of 0

Failed validating 'maximum' in schema[0]:
    {'maximum': 0, 'type': 'number'}

With :

{"value" : [-3]}

Expected result (OK) :

-3 is less than the minimum of 0

Failed validating 'minimum' in schema[1]['items']:
    {'minimum': 0, 'type': 'number'}

Current result

-3 is less than the minimum of 0

Failed validating 'minimum' in schema[1]['items']:
    {'minimum': 0, 'type': 'number'}

With :

{"value": "something"}

Expected result (NOK) :

'check' was expected

Failed validating 'const' in schema[0]:
    {'const': 'check', 'type': 'string'}

Current result :

'something' is not of type 'number'

Failed validating 'type' in schema[0]:
    {'maximum': 0, 'type': 'number'}

On the implementation of such feature, I believe if we were to add a field on _Error to capture if the rule is of the same_type as the instance, and then changing the return function of by_relevance by adding the error.same_type on the tuple being returned, would solve that issue.

I just had a look on how to do it, there are probably better ways of doing this, I am not familiar with the code base.

@Julian Julian added the Enhancement Some new desired functionality label Aug 30, 2020
@miquelougoogle
Copy link

Hey Julian, would you be able to comment on why PR 738 was closed. I appreciate that you may want to do this differently, or that there is some other refactoring in place which would break this. Just looking to understand the current status. Thanks for you time.

@Julian
Copy link
Member

Julian commented Oct 23, 2020

Oy. Sorry @miquelougoogle it's actually nothing of the sort.

I had finished the move of jsonschema's main branch to be called "main" (instead of "master") and deleted the master branch, and I always forget that GitHub does not let you change the target branch of a PR, so it's gone and closed all of the ones that were targeting master and won't let me reopen them.

Apologies for the mixup, there's definitely nothing more to it, fully intend to merge that PR if we get it over the finish line!

@miquelougoogle
Copy link

Ah ! All clear, thanks for the quick reply !

I've re-created the PR to point on the new default branch (main).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Some new desired functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants