feat!: update gks-common / vrs models (#453)

close #452

Author: korikuzma

src/ga4gh/core/domain_models.py

@@ -1,15 +1,4 @@
-"""GKS Common Library Domain Entity models
-
-**This module should not be imported directly.**
-
-Instead, users should use one of the following:
-
-  * `from ga4gh.core import domain_models`, and refer to models with the
-    abbreviated name, e.g., `domain_models.Gene` (recommended)
-
-  * `import ga4gh.core`, and refer to models using the fully-qualified
-    module name, e.g., `ga4gh.core.domain_models.Gene`
-"""
+"""GKS Common Library Domain Entity models"""
 from enum import Enum
 from typing import Literal, Union, List
 
@@ -138,5 +127,5 @@ class Gene(DomainEntity):
 
     type: Literal["Gene"] = Field(
         CommonDomainType.GENE.value,
-        description=f'MUST be "{CommonDomainType.GENE.value}".'
+        description=f'MUST be "{CommonDomainType.GENE.value}"'
     )

src/ga4gh/core/entity_models.py

@@ -32,7 +32,7 @@
 from ga4gh.core.pydantic import (
     getattr_in
 )
-from ga4gh.core.entity_models import IRI, Expression, Entity
+from ga4gh.core.entity_models import IRI, Entity
 
 
 def flatten(vals):
@@ -151,6 +151,13 @@ class VrsType(str, Enum):
     CN_CHANGE = "CopyNumberChange"
 
 
+class Orientation(str, Enum):
+    """The orientation of the molecular variation component."""
+
+    FORWARD = "forward"
+    REVERSE_COMPLEMENT = "reverse_complement"
+
+
 class ResidueAlphabet(str, Enum):
     """The interpretation of the character codes referred to by the refget accession,
     where "aa" specifies an amino acid character set, and "na" specifies a nucleic acid
@@ -174,6 +181,22 @@ class CopyChange(str, Enum):
     EFO_0030072 = 'EFO:0030072'
 
 
+class Syntax(str, Enum):
+    """The syntax used to describe the variation. The value should be one of the
+    supported syntaxes.
+    """
+
+    HGVS_C = "hgvs.c"
+    HGVS_P = "hgvs.p"
+    HGVS_G = "hgvs.g"
+    HGVS_M = "hgvs.m"
+    HGVS_N = "hgvs.n"
+    HGVS_R = "hgvs.r"
+    HGVS_ISCN = "iscn"
+    GNOMAD = "gnomad"
+    SPDI = "spdi"
+
+
 def _recurse_ga4gh_serialize(obj):
     if isinstance(obj, _Ga4ghIdentifiableObject):
         return obj.get_or_create_digest()
@@ -314,6 +337,18 @@ def get_or_create_digest(self, recompute=False) -> str:
     class ga4gh(_ValueObject.ga4gh):
         prefix: str
 
+class Expression(BaseModel):
+    """Representation of a variation by a specified nomenclature or syntax for a
+    Variation object. Common examples of expressions for the description of molecular
+    variation include the HGVS and ISCN nomenclatures.
+    """
+
+    model_config = ConfigDict(use_enum_values=True)
+
+    syntax: Syntax = Field(..., description="The syntax used to describe the variation. The value should be one of the supported syntaxes.")
+    value: str = Field(..., description="The expression of the variation in the specified syntax. The value should be a valid expression in the specified syntax.")
+    syntax_version: Optional[str] = Field(None, description="The version of the syntax used to describe the variation. This is particularly important for HGVS expressions, as the syntax has evolved over time.")
+
 
 #########################################
 # vrs numerics, comparators, and ranges
@@ -392,7 +427,7 @@ class LengthExpression(_ValueObject):
     type: Literal["LengthExpression"] = Field(
         VrsType.LEN_EXPR.value, description=f'MUST be "{VrsType.LEN_EXPR.value}"'
     )
-    length: Optional[Union[Range, int]] = None
+    length: Optional[Union[Range, int]] = Field(None, description="The length of the sequence.")
 
     class ga4gh(_ValueObject.ga4gh):
         keys = [
@@ -408,13 +443,13 @@ class ReferenceLengthExpression(_ValueObject):
         VrsType.REF_LEN_EXPR.value, description=f'MUST be "{VrsType.REF_LEN_EXPR.value}"'
     )
     length: Union[Range, int] = Field(
-        ..., description='The number of residues of the expressed sequence.'
+        ..., description='The number of residues in the expressed sequence.'
     )
     sequence: Optional[SequenceString] = Field(
-        None, description='the `Sequence` encoded by the Reference Length Expression.'
+        None, description='the literal Sequence encoded by the Reference Length Expression.'
     )
     repeatSubunitLength: int = Field(
-        ..., description='The number of residues of the repeat subunit.'
+        ..., description='The number of residues in the repeat subunit.'
     )
 
     class ga4gh(_ValueObject.ga4gh):
@@ -452,9 +487,9 @@ class SequenceReference(_ValueObject):
     type: Literal["SequenceReference"] = Field(VrsType.SEQ_REF.value, description=f'MUST be "{VrsType.SEQ_REF.value}"')
     refgetAccession: Annotated[str, StringConstraints(pattern=r'^SQ.[0-9A-Za-z_\-]{32}$')] = Field(
         ...,
-        description='A `GA4GH RefGet <http://samtools.github.io/hts-specs/refget.html>` identifier for the referenced sequence, using the sha512t24u digest.',
+        description='A [GA4GH RefGet](http://samtools.github.io/hts-specs/refget.html) identifier for the referenced sequence, using the sha512t24u digest.',
     )
-    residueAlphabet: Optional[ResidueAlphabet] = Field(None, description="The interpretation of the character codes referred to by the refget accession, where 'aa' specifies an amino acid character set, and 'na' specifies a nucleic acid character set.")
+    residueAlphabet: Optional[ResidueAlphabet] = Field(None, description='The interpretation of the character codes referred to by the refget accession, where "aa" specifies an amino acid character set, and "na" specifies a nucleic acid character set.')
     circular: Optional[bool] = Field(None, description="A boolean indicating whether the molecule represented by the sequence is circular (true) or linear (false).")
 
     class ga4gh(_ValueObject.ga4gh):
@@ -469,15 +504,15 @@ class SequenceLocation(_Ga4ghIdentifiableObject):
 
     type: Literal["SequenceLocation"] = Field(VrsType.SEQ_LOC.value, description=f'MUST be "{VrsType.SEQ_LOC.value}"')
     sequenceReference: Optional[Union[IRI, SequenceReference]] = Field(
-        None, description='A reference to a `Sequence` on which the location is defined.'
+        None, description='A reference to a Sequence on which the location is defined.'
     )
     start: Optional[Union[Range, int]] = Field(
         None,
-        description='The start coordinate or range of the SequenceLocation. The minimum value of this coordinate or range is 0.',
+        description='The start coordinate or range of the SequenceLocation. The minimum value of this coordinate or range is 0. MUST represent a coordinate or range less than or equal to the value of `end`.',
     )
     end: Optional[Union[Range, int]] = Field(
         None,
-        description='The end coordinate or range of the SequenceLocation. The minimum value of this coordinate or range is 0.',
+        description='The end coordinate or range of the SequenceLocation. The minimum value of this coordinate or range is 0. MUST represent a coordinate or range greater than or equal to the value of `start`.',
 
     )
     sequence: Optional[SequenceString] = Field(None, description="The literal sequence encoded by the `sequenceReference` at these coordinates.")
@@ -614,7 +649,7 @@ class CisPhasedBlock(_VariationBase):
     type: Literal["CisPhasedBlock"] = Field(VrsType.CIS_PHASED_BLOCK.value, description=f'MUST be "{VrsType.CIS_PHASED_BLOCK.value}"')
     members: List[Union[Allele, IRI]] = Field(
         ...,
-        description='A list of `Alleles` that are found in-cis on a shared molecule.',
+        description='A list of Alleles that are found in-cis on a shared molecule.',
         min_length=2,
     )
     sequenceReference: Optional[SequenceReference] = Field(None, description="An optional Sequence Reference on which all of the in-cis Alleles are found. When defined, this may be used to implicitly define the `sequenceReference` attribute for each of the CisPhasedBlock member Alleles.")
@@ -643,7 +678,7 @@ class Adjacency(_VariationBase):
     potentially with an intervening linker sequence.
     """
 
-    type: Literal["Adjacency"] = Field(VrsType.ADJACENCY.value, description=f'MUST be "{VrsType.ADJACENCY.value}"')
+    type: Literal["Adjacency"] = Field(VrsType.ADJACENCY.value, description=f'MUST be "{VrsType.ADJACENCY.value}".')
     adjoinedSequences: List[Union[IRI, SequenceLocation]] = Field(
         ...,
         description="The terminal sequence or pair of adjoined sequences that defines in the adjacency.",
@@ -654,7 +689,7 @@ class Adjacency(_VariationBase):
         None,
         description="The sequence found between adjoined sequences."
     )
-    homology: Optional[bool] = Field(None, description="A flag indicating if coordinate ambiguity in the adjoined sequences is from sequence homology (true) or other uncertainty (false).")
+    homology: Optional[bool] = Field(None, description="A flag indicating if coordinate ambiguity in the adjoined sequences is from sequence homology (true) or other uncertainty, such as instrument ambiguity (false).")
 
     class ga4gh(_Ga4ghIdentifiableObject.ga4gh):
         prefix = 'AJ'
@@ -671,7 +706,7 @@ class Terminus(_VariationBase):
     is not allowed and it removes the unnecessary array structure.
     """
 
-    type: Literal["Terminus"] = Field(VrsType.TERMINUS.value, description=f'MUST be "{VrsType.TERMINUS.value}"')
+    type: Literal["Terminus"] = Field(VrsType.TERMINUS.value, description=f'MUST be "{VrsType.TERMINUS.value}".')
     location: Union[IRI, SequenceLocation] = Field(..., description="The location of the terminus.")
 
     class ga4gh(_Ga4ghIdentifiableObject.ga4gh):
@@ -685,17 +720,19 @@ class TraversalBlock(_ValueObject):
     """A component used to describe the orientation of a molecular variation within
     a DerivativeMolecule."""
 
+    model_config = ConfigDict(use_enum_values=True)
+
     type: Literal["TraversalBlock"] = Field(
-        VrsType.TRAVERSAL_BLOCK.value, description=f'MUST be "{VrsType.TRAVERSAL_BLOCK.value}"'
+        VrsType.TRAVERSAL_BLOCK.value, description=f'MUST be "{VrsType.TRAVERSAL_BLOCK.value}".'
     )
-    orientation: Literal["forward", "reverse_complement"] = Field(
-        ...,
-        description='The orientation of the traversal block, either forward or reverse_complement.'
+    orientation: Optional[Orientation] = Field(
+        None,
+        description='The orientation of the molecular variation component.'
     )
 
-    component: Union[IRI, Adjacency, Allele, Terminus, CisPhasedBlock] = Field(
-        ...,
-        description="The component that make up the derivative molecule."
+    component: Optional[Union[Allele, CisPhasedBlock, Adjacency, Terminus]] = Field(
+        None,
+        description="The unoriented molecular variation component."
     )
 
     class ga4gh(_ValueObject.ga4gh):
@@ -710,13 +747,13 @@ class DerivativeMolecule(_VariationBase):
     molecule composed from multiple sequence components.
     """
 
-    type: Literal["DerivativeMolecule"] = Field(VrsType.DERIVATIVE_MOL.value, description=f'MUST be "{VrsType.DERIVATIVE_MOL.value}"')
-    components: List[TraversalBlock] = Field(
+    type: Literal["DerivativeMolecule"] = Field(VrsType.DERIVATIVE_MOL.value, description=f'MUST be "{VrsType.DERIVATIVE_MOL.value}".')
+    components: List[Union[IRI, TraversalBlock]] = Field(
         ...,
-        description="The traversal block components that make up the derivative molecule.",
+        description="The molecular components that constitute the derivative molecule.",
         min_length=2
     )
-    circular: Optional[bool] = Field(None, description="A flag indicating if the derivative molecule is circular (true) or linear (false).")
+    circular: Optional[bool] = Field(None, description="A boolean indicating whether the molecule represented by the sequence is circular (true) or linear (false).")
 
     class ga4gh(_Ga4ghIdentifiableObject.ga4gh):
         prefix = "DM"
@@ -769,7 +806,7 @@ class CopyNumberChange(_CopyNumber):
     type: Literal["CopyNumberChange"] = Field(VrsType.CN_CHANGE.value, description=f'MUST be "{VrsType.CN_CHANGE.value}"')
     copyChange: CopyChange = Field(
         ...,
-        description='MUST be one of "EFO:0030069" (complete genomic loss), "EFO:0020073" (high-level loss), "EFO:0030068" (low-level loss), "EFO:0030067" (loss), "EFO:0030064" (regional base ploidy), "EFO:0030070" (gain), "EFO:0030071" (low-level gain), "EFO:0030072" (high-level gain).',
+        description='MUST be a Coding representing one of "EFO:0030069" (complete genomic loss), "EFO:0020073" (high-level loss), "EFO:0030068" (low-level loss), "EFO:0030067" (loss), "EFO:0030064" (regional base ploidy), "EFO:0030070" (gain), "EFO:0030071" (low-level gain), "EFO:0030072" (high-level gain).',
     )
 
     class ga4gh(_Ga4ghIdentifiableObject.ga4gh):

src/ga4gh/vrs/models.py

@@ -0,0 +1,148 @@
+"""Test that VRS-Python Pydantic models match VRS and GKS-Common schemas"""
+
+from enum import Enum
+import json
+from pathlib import Path
+
+import pytest
+from pydantic import BaseModel
+
+from ga4gh.core import entity_models, domain_models
+from ga4gh.vrs import models as vrs_models
+
+
+class GKSSchema(str, Enum):
+    """Enum for GKS schema"""
+
+    VRS = "vrs"
+    CORE_IM = "core-im"
+    DOMAIN = "domain-entities"
+
+
+class GKSSchemaMapping(BaseModel):
+    """Model for representing GKS Schema concrete classes, primitives, and schema"""
+
+    base_classes: set = set()
+    concrete_classes: set = set()
+    primitives: set = set()
+    schema: dict = {}
+
+
+def _update_gks_schema_mapping(
+    f_path: Path, gks_schema_mapping: GKSSchemaMapping
+) -> None:
+    """Update ``gks_schema_mapping`` properties
+
+    :param f_path: Path to JSON Schema file
+    :param gks_schema_mapping: GKS schema mapping to update
+    """
+    with f_path.open() as rf:
+        cls_def = json.load(rf)
+
+    spec_class = cls_def["title"]
+    gks_schema_mapping.schema[spec_class] = cls_def
+
+    if "properties" in cls_def:
+        gks_schema_mapping.concrete_classes.add(spec_class)
+    elif cls_def.get("type") in {"array", "integer", "string"}:
+        gks_schema_mapping.primitives.add(spec_class)
+    else:
+        gks_schema_mapping.base_classes.add(spec_class)
+
+
+GKS_SCHEMA_MAPPING = {gks: GKSSchemaMapping() for gks in GKSSchema}
+SUBMODULES_DIR = Path(__file__).parents[2] / "submodules" / "vrs"
+
+
+# Get vrs classes
+vrs_mapping = GKS_SCHEMA_MAPPING[GKSSchema.VRS]
+for f in (SUBMODULES_DIR / "schema" / "vrs" / "json").glob("*"):
+    _update_gks_schema_mapping(f, vrs_mapping)
+
+
+# Get core-im + domain classes
+for child in (SUBMODULES_DIR / "submodules" / "gks-common" / "schema").iterdir():
+    mapping_key = (
+        GKSSchema.DOMAIN if str(child).endswith(GKSSchema.DOMAIN) else GKSSchema.CORE_IM
+    )
+    mapping = GKS_SCHEMA_MAPPING[mapping_key]
+    for f in (child / "json").glob("*"):
+        _update_gks_schema_mapping(f, mapping)
+
+
+@pytest.mark.parametrize(
+    ("gks_schema", "pydantic_models"),
+    [
+        (GKSSchema.VRS, vrs_models),
+        (GKSSchema.CORE_IM, entity_models),
+        (GKSSchema.DOMAIN, domain_models),
+    ],
+)
+def test_schema_models_in_pydantic(gks_schema, pydantic_models):
+    """Ensure that each schema model has corresponding Pydantic model"""
+    mapping = GKS_SCHEMA_MAPPING[gks_schema]
+    for schema_model in (
+        mapping.base_classes | mapping.concrete_classes | mapping.primitives
+    ):
+        assert getattr(pydantic_models, schema_model, False), schema_model
+
+
+@pytest.mark.parametrize(
+    ("gks_schema", "pydantic_models"),
+    [
+        (GKSSchema.VRS, vrs_models),
+        (GKSSchema.CORE_IM, entity_models),
+        (GKSSchema.DOMAIN, domain_models),
+    ],
+)
+def test_schema_class_fields(gks_schema, pydantic_models):
+    """Check that each schema model properties exist and are required in corresponding
+    Pydantic model, and validate required properties
+    """
+    mapping = GKS_SCHEMA_MAPPING[gks_schema]
+    for schema_model in mapping.concrete_classes:
+        schema_properties = mapping.schema[schema_model]["properties"]
+        pydantic_model = getattr(pydantic_models, schema_model)
+        assert set(pydantic_model.model_fields) == set(schema_properties), schema_model
+
+        required_schema_fields = set(mapping.schema[schema_model]["required"])
+
+        for prop, property_def in schema_properties.items():
+            pydantic_model_field_info = pydantic_model.model_fields[prop]
+            pydantic_field_required = pydantic_model_field_info.is_required()
+
+            if prop in required_schema_fields:
+                if prop != "type":
+                    assert pydantic_field_required, f"{pydantic_model}.{prop}"
+            else:
+                assert not pydantic_field_required, f"{pydantic_model}.{prop}"
+
+            if "description" in property_def:
+                assert property_def["description"].replace("'", "\"") == pydantic_model_field_info.description.replace("'", "\""), f"{pydantic_model}.{prop}"
+            else:
+                assert pydantic_model_field_info.description is None, f"{pydantic_model}.{prop}"
+
+
+def test_ga4gh_keys():
+    """Ensure ga4ghDigest keys defined in schema model exist in corresponding Pydantic model"""
+    vrs_mapping = GKS_SCHEMA_MAPPING[GKSSchema.VRS]
+    for vrs_class in vrs_mapping.concrete_classes:
+        if (
+            vrs_mapping.schema[vrs_class].get("ga4ghDigest", {}).get("keys", None)
+            is None
+        ):
+            continue
+
+        pydantic_model = getattr(vrs_models, vrs_class)
+
+        try:
+            pydantic_model_digest_keys = pydantic_model.ga4gh.keys
+        except AttributeError as e:
+            raise AttributeError(vrs_class) from e
+
+        assert set(pydantic_model_digest_keys) == set(
+            vrs_mapping.schema[vrs_class]["ga4ghDigest"]["keys"]
+        ), vrs_class
+        assert pydantic_model_digest_keys == sorted(
+            pydantic_model.ga4gh.keys
+        ), vrs_class