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

Add SV Genotype Example #463

Draft
wants to merge 4 commits into
base: 2.x
Choose a base branch
from

Conversation

Mrinal-Thomas-Epic
Copy link

@Mrinal-Thomas-Epic Mrinal-Thomas-Epic commented Feb 2, 2024

Changes

  • Add SV genotype example to tests, along with image
    • I did not include the yaml in the image, since it will be easier to maintain the images over time if they only show the concept being tested and not the exact syntax. I can apply this to other test images if you agree.
    • I allowed Adjacency to be specified directly on GenotypeMember.variation. We already allow this compact syntax for Allele, so I think it makes sense to extend it to Adjacency.
    • Feedback requested: I added Adjacency to MolecularVariation, since Adjacency inherits from it and seems to match the definition. However, I'm not familiar with all the use cases of the MolecularVariation class, so let me know if this should be reverted
    • Feedback requested: The SV genotype example uses the compact sequence location syntax, by excluding the SequenceLocation.sequenceReference on adjoinedSequences in the second genotype member. I have seen this syntax used within a Haplotype before but not a Genotype. Can you confirm that this is allowed on a Genotype?
  • Move images referenced by tests to a subfolder for organization
  • Fix small bug where tests were attempting to catch the wrong type of exception
  • Fix required python version (see gks-metaschema #1)

This python version is required for the YAMLSchemaProcessor in ga4gh.gks.metaschema.tools.source_proc.
Remove extra parent directory symbol and move test images to their own folder
Fix a bug where tests looked for AssertionError, but jsonschema raises a ValidationError.
@larrybabb
Copy link
Contributor

@ahwagner what's the plan for this PR? I think you've covered all this in your other changes and we've decided to omit Genotype and GenotypeMember (I think - or should that be left in draft?). We should make sure to close this out if it's been covered by other updates.

@ahwagner ahwagner marked this pull request as draft March 5, 2024 03:16
@ahwagner
Copy link
Member

ahwagner commented Mar 5, 2024

@larrybabb we haven't yet discussed new Genotypes in the VR calls. I don't want to close this PR since it serves as a useful showcase for what @Mrinal-Thomas-Epic wants to discuss. I think the first step, though, is for us to address the Haplotype model, and then discuss this issue with the VR community, and then update this PR accordingly.

@Mrinal-Thomas-Epic if you want to raise this topic on an upcoming VRS-SV call, that would be a fine introduction as well–but I ask that you start by making a discussion outlining the goals and share that discussion with the SIG and/or broader VR group in advance. You can reference this PR in that discussion to help orient people to your draft proposal–it's a great way to get community feedback!

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.

None yet

3 participants