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

Enum must be an array and other schema failures #71

Merged
merged 4 commits into from
Sep 17, 2020
Merged

Enum must be an array and other schema failures #71

merged 4 commits into from
Sep 17, 2020

Conversation

jnothman
Copy link
Contributor

Fixes #70

@hlydecker
Copy link
Contributor

Interesting new error:

[ { keyword: 'pattern',
    dataPath: '.agcontexts[0].bbch_code',
    schemaPath:
     'https://weedid.sydney.edu.au/schema/AgContext.json#//properties/bbch_code/pattern',
    params: { pattern: '^gs(?!00)[0-9][0-9]$|^na$' },
    message: 'should match pattern "^gs(?!00)[0-9][0-9]$|^na$"' } ]
../conversion_tools/deepweeds_to_json/deepweeds_imageinfo.json valid
##[error]Process completed with exit code 1.

@jnothman
Copy link
Contributor Author

Ahhh I was confused by you missing out the line "../conversion_tools/cwfid_to_json/cwfid_imageinfo.json invalid". cwfid has bbch_code = "GS10-19".

@hlydecker
Copy link
Contributor

Yeah both this and the previous issue with the datum object are a symptom of the same disease: updates occurring off master but not being merged in because they are part of a larger branch.

@jnothman
Copy link
Contributor Author

Yeah both this and the previous issue with the datum object are a symptom of the same disease: updates occurring off master but not being merged in because they are part of a larger branch.

I don't understand exactly. You saying that one branch was merged and another branch was merged, and their integration was not tested with CI?

The issue here, though, is that CWFID does actually have an invalid bbch_code. What should it be?

hlydecker pushed a commit that referenced this pull request Sep 16, 2020
fixes issues with #70 and #71
@hlydecker
Copy link
Contributor

Yeah both this and the previous issue with the datum object are a symptom of the same disease: updates occurring off master but not being merged in because they are part of a larger branch.

I don't understand exactly. You saying that one branch was merged and another branch was merged, and their integration was not tested with CI?

The issue here, though, is that CWFID does actually have an invalid bbch_code. What should it be?

Oh I think I didn't understand what was happening. Scratch what I said.

Re the bbch_code, I changed that. The old one we had was a hold over from a previous representation of bbch_code.

@jnothman
Copy link
Contributor Author

It expects lowercase (for good or bad)

@hlydecker
Copy link
Contributor

I saw that and changed to lower case; still doesn't work. However in going back to the CWFID paper just to make sure I had recorded it right, I reread their description of crop stage:

The growth stage of the
crop was approximately BBCH 10 – 20 (see [15] for a description of the BBCH
plant growth stage scale) and a significant amount of close-to-crop and intra-row
weeds was present.

We currently have it set to just allow one BBCH code or NA... but should we support a range of BBCH codes?

@jnothman
Copy link
Contributor Author

We currently have it set to just allow one BBCH code or NA... but should we support a range of BBCH codes?

You need to speak to the Ag people about that... And think about how you would search for data in a particular range. We could also consider specific ranges, e.g. GS1x. I'm not sure whether their specification of 10-20 is inclusive of 20. I suppose so?

@jnothman jnothman changed the title Enum must be an array Enum must be an array and other schema failures Sep 17, 2020
@jnothman jnothman merged commit 88f2fb1 into master Sep 17, 2020
@jnothman jnothman deleted the fix60 branch September 17, 2020 01:48
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.

Validate JSON failing on master
2 participants