diff --git a/src/ga4gh/vrs/models.py b/src/ga4gh/vrs/models.py index ad2978d5..8acdf33e 100644 --- a/src/ga4gh/vrs/models.py +++ b/src/ga4gh/vrs/models.py @@ -11,6 +11,7 @@ module name, e.g., `ga4gh.vrs.models.Allele` """ +import copy import inspect import sys from abc import ABC @@ -327,7 +328,10 @@ def get_or_create_ga4gh_identifier( - 'always': this will update the vro.id field any time the identifier is computed - 'never': the vro.id field will not be edited in-place, - even when empty + even when empty. The vro object is not mutated in any way: + digest computation is performed on a deep copy, so neither + the vro.digest field nor digest fields on nested objects + are set. Digests will be recalculated even if present if recompute is True. @@ -344,7 +348,10 @@ def get_or_create_ga4gh_identifier( elif in_place == "always": self.id = self.compute_ga4gh_identifier(recompute) elif in_place == "never": - return self.compute_ga4gh_identifier(recompute) + # Digest computation stores digests on the object and its nested + # identifiable objects, so compute on a deep copy to leave the + # caller's object (including its digest fields) untouched. + return copy.deepcopy(self).compute_ga4gh_identifier(recompute) else: msg = "Expected 'in_place' to be one of 'default', 'always', or 'never'" raise ValueError(msg) diff --git a/tests/test_vrs.py b/tests/test_vrs.py index 975854ab..add14df4 100644 --- a/tests/test_vrs.py +++ b/tests/test_vrs.py @@ -194,6 +194,44 @@ def test_cpb(): assert ga4gh_identify(cpb_431012) == "ga4gh:CPB.x8GH5G73cPMs37jy1-9mJjWynu324rxI" +def test_identify_in_place_never_does_not_mutate(): + """ga4gh_identify(..., in_place="never") must not mutate the input object. + + Regression test for https://github.com/ga4gh/vrs-python/issues/440: + digest computation used to set `digest` fields on the object and its + nested identifiable objects even when in_place="never". + """ + allele = models.Allele(**allele_dict) + before = allele.model_dump_json(exclude_none=True) + assert allele.digest is None + assert allele.location.digest is None + + obj_id = ga4gh_identify(allele, in_place="never") + + assert obj_id == "ga4gh:VA.Hy2XU_-rp4IMh6I_1NXNecBo8Qx8n0oE" + assert allele.id is None + assert allele.digest is None + assert allele.location.digest is None + assert allele.model_dump_json(exclude_none=True) == before + + +def test_identify_in_place_modes_still_mutate(): + """Sanity check: in_place="default"/"always" keep their mutating behavior.""" + allele = models.Allele(**allele_dict) + assert ( + ga4gh_identify(allele, in_place="default") + == "ga4gh:VA.Hy2XU_-rp4IMh6I_1NXNecBo8Qx8n0oE" + ) + assert allele.id == "ga4gh:VA.Hy2XU_-rp4IMh6I_1NXNecBo8Qx8n0oE" + + allele = models.Allele(**allele_dict) + assert ( + ga4gh_identify(allele, in_place="always") + == "ga4gh:VA.Hy2XU_-rp4IMh6I_1NXNecBo8Qx8n0oE" + ) + assert allele.id == "ga4gh:VA.Hy2XU_-rp4IMh6I_1NXNecBo8Qx8n0oE" + + def test_ga4gh_iri(): iri = models.iriReference.model_construct( "ga4gh:VA.Hy2XU_-rp4IMh6I_1NXNecBo8Qx8n0oE"