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

Measurements schema #859

Merged
merged 2 commits into from
Mar 7, 2022
Merged

Measurements schema #859

merged 2 commits into from
Mar 7, 2022

Conversation

joverlee521
Copy link
Contributor

Add measurements JSON schema and add as a subcommand to augur validate.
Schema file name matches the file name for the redirect in nextstrain/nextstrain.org#481

I'm expecting the new augur measurements command to be a bigger change requiring additional discussion and reviews, so I'll leave that for a later PR.

@codecov
Copy link

codecov bot commented Mar 3, 2022

Codecov Report

Merging #859 (bcd36df) into master (1b56153) will decrease coverage by 0.01%.
The diff coverage is 16.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #859      +/-   ##
==========================================
- Coverage   34.32%   34.31%   -0.02%     
==========================================
  Files          41       41              
  Lines        5931     5937       +6     
  Branches     1518     1518              
==========================================
+ Hits         2036     2037       +1     
- Misses       3813     3818       +5     
  Partials       82       82              
Impacted Files Coverage Δ
augur/validate.py 40.59% <16.66%> (-1.52%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1b56153...bcd36df. Read the comment docs.

@joverlee521 joverlee521 requested a review from a team March 3, 2022 00:29
Copy link
Member

@tsibley tsibley left a comment

Choose a reason for hiding this comment

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

LGTM! Just one typo and a suggested change to avoid unnecessary patternProperties use.

@@ -132,6 +139,8 @@ def register_arguments(parser):
subparsers.add_parser("auspice-config-v2", help="validate auspice config intended for `augur export v2`") \
.add_argument('config_json', metavar='JSON', help="auspice config JSON")

subparsers.add_parser("measurements", help="validate measuremens JSON intended for auspice measurements panel") \
Copy link
Member

Choose a reason for hiding this comment

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

measuremens → measurements

Comment on lines 140 to 143
"patternProperties": {
"^.*$": {
"description": "Metadata associated with the measurement. Only metadata properties included in the groupings config will be included in the group by dropdown, but all metadata properties will be available as a filter",
"type": ["string", "number", "boolean"]
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"patternProperties": {
"^.*$": {
"description": "Metadata associated with the measurement. Only metadata properties included in the groupings config will be included in the group by dropdown, but all metadata properties will be available as a filter",
"type": ["string", "number", "boolean"]
}
}
"additionalProperties": {
"description": "Metadata associated with the measurement. Only metadata properties included in the groupings config will be included in the group by dropdown, but all metadata properties will be available as a filter",
"type": ["string", "number", "boolean"]
}

Based on the data structure that Auspice version 2.34.0 expects for the
measurements sidecar JSON.
Validates the provided measurements JSON against the JSON schema
`augur/data/schema-measurements.json`
@joverlee521 joverlee521 force-pushed the measurements-schema branch from 8817c9b to bcd36df Compare March 7, 2022 22:23
@joverlee521 joverlee521 merged commit e4fdf59 into master Mar 7, 2022
@joverlee521 joverlee521 deleted the measurements-schema branch March 7, 2022 23:22
@victorlin victorlin added this to the Next release X.X.X milestone Mar 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

3 participants