Repository navigation
merge: migrate main into v3 - #676
Merged
Merged
Conversation
_from_vrs subscripted the ga4gh.vrs.models module with var["type"],
raising TypeError ('module' object is not subscriptable) for any VRS dict
input. Resolve the model class with getattr(models, var["type"], None)
and return None gracefully for unknown types, preserving the original
intent of the KeyError guard.
Closes #489
pysam 0.24.0 fixes the Python 3.13 + pytest-cov failure (pysam-developers/pysam#1350) that the pysam==0.23.0 pin worked around (#560). Closes #653
Closes #653. Replaces the `pysam==0.23.0` pin (#560) with `pysam>=0.24.0`. pysam 0.23.1–0.23.3 wheels were built with Cython 3.1, which broke `pytest-cov` on Python 3.13 (`ValueError: Firing event 10 with no exception set` when iterating a `VariantFile`; pysam-developers/pysam#1350). pysam 0.24.0 fixed this. Verified the full suite passes with coverage on Python 3.10–3.13 with pysam 0.24.0 and 0.24.1, and that VCF annotation output is unchanged from 0.23.0.
## Bug Translating FROM VRS (a dict VRS object) via `_Translator._from_vrs` crashed with `TypeError: 'module' object is not subscriptable`. The code subscripted `models` (the `ga4gh.vrs.models` module) with `var["type"]`. ## Root cause In `src/ga4gh/vrs/extras/translator.py`, `_from_vrs` did `model = models[var["type"]]`. Modules are not subscriptable, so any VRS dict input crashed. The surrounding try/except KeyError was meant to return None for unknown types, but the TypeError was never caught. ## Fix Resolve the model class with `getattr(models, var["type"], None)` and return None gracefully when the type is unknown, preserving the original intent. Scope is the crash only; the broader translator redesign discussed in the thread is left to maintainers. ## Verification * Reproduced the crash on pristine upstream main before the fix: `translate_from(vrs_dict, fmt="vrs")` raised TypeError at translator.py line 170. * Added regression test `test_from_vrs_dict` in `tests/extras/test_allele_translator.py`: a valid VRS dict now translates to an `Allele` without TypeError; unknown types, non dict input, and missing type all return None. * After the fix the new test passes, and the valid dict round trips to a models.Allele. ## What I could not verify * The full existing translator test suite cannot run green in this sandbox: 20 tests in test_allele_translator.py and 7 in test_cnv_translator.py fail identically before and after this change because they need live network access to uta.biocommons.org and a local seqrepo REST server, which are unavailable here. No new failures were introduced by this change. * CI on this PR has not been observed; I am not claiming it is green. Closes #489
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.
Prevents test_rle_output.pkl and test_rle_output.vcf from being written to tests/extras/data dir
#656 fixed the `TypeError` in `_from_vrs`, but its `getattr(models, var["type"])` lookup accepts any attribute of the `models` module. Inputs like `{"type": "SequenceLocation", ...}` or `{"type": "Field"}` return non-variation objects. This narrows the accepted types to VRS variations by validating against the existing `models.Variation` union (tagged by `type`, mirroring the VRS `Variation` schema). Other types now raise a pydantic `ValidationError` listing the accepted types.
theferrit32
approved these changes
Oct 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.