From 39fb5c2faf1e6119e72f5729993e414ef623988a Mon Sep 17 00:00:00 2001 From: Kyle Ferriter Date: Tue, 6 Oct 2026 00:33:43 -0400 Subject: [PATCH] fix(translator): reject non-variation types in _from_vrs Validate VRS dict input against the models.Variation union instead of looking up any attribute of the models module by name. Types that are not VRS variations (e.g. SequenceLocation, or unrelated module attributes like Field) now raise a pydantic ValidationError listing the accepted types, rather than building a non-variation object or returning None. --- src/ga4gh/vrs/extras/translator.py | 10 ++++----- tests/extras/test_allele_translator.py | 30 +++++++++++++++++++++----- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/src/ga4gh/vrs/extras/translator.py b/src/ga4gh/vrs/extras/translator.py index b10b62b6..269c2598 100644 --- a/src/ga4gh/vrs/extras/translator.py +++ b/src/ga4gh/vrs/extras/translator.py @@ -161,15 +161,15 @@ def hgvs_tools(self) -> HgvsTools: return HgvsTools(self.data_proxy) def _from_vrs(self, var: dict, **kwargs) -> models._VariationBase | None: # noqa: ARG002 - """Convert from dict representation of VRS JSON to VRS object""" + """Convert from dict representation of VRS JSON to VRS object + + :raise pydantic.ValidationError: If `var` is not a valid VRS variation + """ if not isinstance(var, Mapping): return None if "type" not in var: return None - model = getattr(models, var["type"], None) - if model is None: - return None - return model(**var) + return models.Variation.model_validate(var).root class AlleleTranslator(_Translator): diff --git a/tests/extras/test_allele_translator.py b/tests/extras/test_allele_translator.py index 02f0d56c..e5e48d34 100644 --- a/tests/extras/test_allele_translator.py +++ b/tests/extras/test_allele_translator.py @@ -1,4 +1,5 @@ import pytest +from pydantic import ValidationError from ga4gh.vrs import models from ga4gh.vrs.dataproxy import DataProxyValidationError @@ -987,9 +988,8 @@ def test_translate_to_invalid_fmt(tlr): def test_from_vrs_dict(): """Regression test for ga4gh/vrs-python#489. - Translating a VRS dict must resolve the model class from the `models` - module via getattr (modules are not subscriptable) instead of crashing - with TypeError; unknown types return None gracefully. + Translating a VRS dict must resolve the model class from its `type` + instead of crashing with TypeError (the `models` module was subscripted). """ tlr = AlleleTranslator(data_proxy=None, identify=False) @@ -1000,8 +1000,28 @@ def test_from_vrs_dict(): assert allele.location.start == snv_output["location"]["start"] assert allele.location.end == snv_output["location"]["end"] - # unknown type returns None rather than raising - assert tlr._from_vrs({"type": "NotARealModel"}) is None # non-dict and missing-type inputs still return None assert tlr._from_vrs("NC_000019.10:g.44908822C>T") is None assert tlr._from_vrs({"location": {}}) is None + + +from_vrs_non_variation_type_cases = [ + {"id": "unknown-name", "type": "NotARealModel"}, + {"id": "non-variation-vrs-class", "type": "SequenceLocation"}, + {"id": "non-model-module-attribute", "type": "Field"}, + {"id": "variation-base-class", "type": "_VariationBase"}, + {"id": "non-string", "type": 5}, +] + + +@pytest.mark.parametrize( + "case", from_vrs_non_variation_type_cases, ids=lambda c: c["id"] +) +def test_from_vrs_non_variation_type(case): + """A VRS dict whose `type` is not a VRS variation class is rejected, rather than + resolved to whatever `ga4gh.vrs.models` attribute has that name + """ + tlr = AlleleTranslator(data_proxy=None, identify=False) + with pytest.raises(ValidationError) as e: + tlr.translate_from({"type": case["type"]}, fmt="vrs") + assert e.value.errors()[0]["type"] == "union_tag_invalid"