-
Notifications
You must be signed in to change notification settings - Fork 1
feat!: add base and aac-2017 models from 1.0.0-ballot.2024-11.2 release #14
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
Conversation
quinnwai
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few comments! One question and possibly a bug?
| locusAlleleCount=34086, | ||
| cohort=StudyGroup(id="ALL", label="Overall"), | ||
| ) | ||
| assert caf.focusAllele.root == "allele.json#/1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just clarifying, so I should always use .root for accessing the string for an iriReference? I'm putting in a VRS Allele ID in there, not sure if I'm using it right.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just clarifying, so I should always use .root for accessing the string for an iriReference?
Yes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you give the example you're using?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it would look something like this (model dumped, excluding None):
{
"type": "CohortAlleleFrequencyStudyResult",
"sourceDataSet": {
"id": "../tests/fixtures/1kGP.chr1.1000.vrs.vcf.gz",
"type": "DataSet",
"description": "Created 2025-01-24 14:34:54.383668"
},
"focusAllele": "ga4gh:VA.1WRXw4TC5DYsjC2QLyKux9C0xETLN3Yt",
"focusAlleleCount": 248,
"locusAlleleCount": 1184,
"focusAlleleFrequency": 0.20945945945945946,
"cohort": {
"id": "USA",
"type": "StudyGroup",
"label": "USA"
},
"ancillaryResults": {
"homozygotes": 1,
"hemizygotes": 246,
"phenotypes": [
"USA"
]
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok! In that case, it could be helpful to users to have on the iriReference docs that the string value is nested in .root!
|
@larrybabb I think we should merge this and make a dev release for @quinnwai to use. I can then make a new dev release with breaking changes from 2025 pre-release snapshots. @quinnwai you'll want to make sure you pin the exact version in GKS AnVIL because we expect many breaking changes in the dev releases. |
larrybabb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 LGTM
close #11