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

Fix type maxiumum->maximum on linearlyReferencedPosition #73

Merged
merged 2 commits into from
Dec 13, 2023

Conversation

DavidKarlas
Copy link
Collaborator

@DavidKarlas DavidKarlas commented Nov 3, 2023

Looks like test on https://github.com/OvertureMaps/schema/blob/main/examples/transportation/segment/road/lanes/lanes-on-straight-road-lr.yaml is failing now, because it has values 0-100 and not 0-1, can someone from transportation figure this one out?

Copy link
Contributor

@sanjibdutta1 sanjibdutta1 left a comment

Choose a reason for hiding this comment

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

Did you run the tests? Since tests references these schema elements, some may fail. Please run the tests once

Copy link
Collaborator

@vcschapp vcschapp left a comment

Choose a reason for hiding this comment

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

This is failing automatic validation. Please run ./test.sh locally before publishing the PR, or read the automatic check output on the PR...

example instance 'examples/transportation/segment/road/lanes/lanes-on-straight-road-lr.yaml' is EXPECTED to pass validation but ACTUALLY it failed.

The reason for this is found in the validation output. Line 473-482:

"instanceLocation": "/properties/road/lanes/1/at/0",
"errors": [
  {
    "valid": false,
    "keywordLocation": "/oneOf/4/then/$ref/properties/properties/allOf/0/then/properties/road/$ref/properties/lanes/oneOf/1/items/allOf/0/$ref/properties/at/$ref/items/$ref/maximum",
    "absoluteKeywordLocation": "file:///home/runner/work/schema/schema/schema/defs.yaml#/$defs/propertyDefinitions/linearlyReferencedPosition/maximum",
    "instanceLocation": "/properties/road/lanes/1/at/0",
    "error": "must be \u003c= 1 but found 10"
  }
]

Line 488-497:

"instanceLocation": "/properties/road/lanes/1/at/1",
"errors": [
 {
   "valid": false,
   "keywordLocation": "/oneOf/4/then/$ref/properties/properties/allOf/0/then/properties/road/$ref/properties/lanes/oneOf/1/items/allOf/0/$ref/properties/at/$ref/items/$ref/maximum",
   "absoluteKeywordLocation": "file:///home/runner/work/schema/schema/schema/defs.yaml#/$defs/propertyDefinitions/linearlyReferencedPosition/maximum",
   "instanceLocation": "/properties/road/lanes/1/at/1",
   "error": "must be \u003c= 1 but found 70"
 }
]

etc., etc.

Please fix and update the PR.....

@vcschapp
Copy link
Collaborator

vcschapp commented Dec 6, 2023

Looks like test on https://github.com/OvertureMaps/schema/blob/main/examples/transportation/segment/road/lanes/lanes-on-straight-road-lr.yaml is failing now, because it has values 0-100 and not 0-1, can someone from transportation figure this one out?

It's just an example file - can't you just edit the file? I'd suggest scaling down the maximum values so they fit in 0-1. If they're all in 0-100, maybe divide by 100?

I very much appreciate this schema fix, but I think we're going to have to demonstrate more end-to-end ownership if we want to scale as a group. If you make a one-character deletion to fix a bug, isn't it also fair to run the tests and bring any incorrect test files into sync?

@DavidKarlas DavidKarlas force-pushed the users/davidkarlas/fixMaxiumum branch from 2658cf0 to f9ffe8f Compare December 6, 2023 03:46
@DavidKarlas
Copy link
Collaborator Author

It was probably stupid from me to not just fix examples, but at time I though I'm misunderstanding something and was not sure anymore. I update examples so tests pass now.
Sorry,
David

@RobSoetewey-TomTom RobSoetewey-TomTom merged commit d2d6323 into dev Dec 13, 2023
2 checks passed
@RobSoetewey-TomTom RobSoetewey-TomTom deleted the users/davidkarlas/fixMaxiumum branch December 13, 2023 16:15
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.

7 participants