Repository navigation
fix: format AAExt per HGVS spec 21.1.3 (drop embedded aaterm) - #890
developer-rpai wants to merge 5 commits into
Conversation
N-terminal extensions now format as Met1ext-5 (aaterm is not part of the spec syntax; this supersedes the legacy Met1extMet-5 form). C-terminal extensions keep the Ter110GlnextTer17 shape; a multi-residue aaterm tail sequence is reduced to the aa replacing the stop codon and the new terminator. Closes biocommons#804
|
This issue is stale because it has been open 180 days with no activity. |
|
@reece gentle ping on this one — it closes #804. |
|
Thanks for this contribution. Can we make sure the updated pdots pass a full-circle test also when parsed again as strings? It feels like this is only part of the required change and the parser also needs a corresponding upgrade. |
…cle round-trip test The formatter now emits spec 21.1.3 protein extensions (Met1ext-2, Ter10LeuextTer2), but the parser rejected the multi-residue aaterm tails that older formatters produced (e.g. Met1extLeuGlyMet-2, Ter10GlnextLG*17). Extend aa13_ext in both grammar backends (hgvs.pymeta + regenerated ometa code, and the parallel pyparsing grammar) with alternatives for multi-residue tails: aat13_seq + nnum (N-terminal) and aat13_seq + num (C-terminal, explicit length required so bare extGlyTrp stays invalid). The formatter's spec-slot reduction normalizes these on output, so legacy strings now parse and round-trip to the spec form. Add test_AAExt_issue804_roundtrip: format -> parse -> format fixed-point checks for the issue's examples, spec forms, and legacy normalizations.
|
Good catch on both counts. Pushed a second commit (d58fc56) that does exactly that:
All edit and parser tests pass locally and the SonarCloud quality gate is green. Let me know if you'd like anything adjusted. |
reece
left a comment
reece
left a comment
There was a problem hiding this comment.
Nice work @developer-rpai! Thank you for the contribution!
I made one minor request for change for consistency. Otherwise I think this is ready to merge.
| self.assertEqual(str(hgvs.edit.AAExt("A", "V", None, -10).type), "ext") | ||
| self.assertEqual(str(hgvs.edit.AAExt("A", None, None, -5).type), "ext") | ||
|
|
||
| def test_AAExt_issue804(self): |
There was a problem hiding this comment.
Please move to tests/issues/test_804.py, mostly for consistency with other issue-oriented tests.
Per review feedback, relocate test_AAExt_issue804 and test_AAExt_issue804_roundtrip from tests/test_hgvs_edit.py to the dedicated issue test module, converted to pytest style.
|
This branch has not been deployed
3 participants
Closes #804
Bug
AAExt.format()embedded the rawaatermstring betweenextand the length, producing descriptions that violate HGVS nomenclature spec 21.1.3 (https://hgvs-nomenclature.org/stable/recommendations/protein/extension/):PosEdit(Interval(Met1, Met1), AAExt(ref="M", aaterm="LGM", length=-2))formatted asMet1extLeuGlyMet-2; the spec form isMet1ext-2(the spec explicitly superseded the legacyp.Met1extMet-5form withp.Met1ext-5, soaatermhas no slot in the N-terminal syntax).PosEdit(Interval(Ter10, Ter10), AAExt(ref="*", aaterm="LG*", length=2))formatted asTer10extLeuGlyTer2; the spec form isTer10LeuextTer2(":p.Ter" aa_position aa "extTer" extension_length).Root cause
AAExt.format()insrc/hgvs/edit.pybuilt the string asf"{alt}ext{aaterm}{length}"with no regard for which spec slotsaatermmay occupy.Fix
This is a spec-compliance fix to a core data model's serialization, not a cosmetic tweak. The change is confined to
AAExt.format():length):aatermis dropped, formatting as{alt}ext{length}. The optionalaltprefix is preserved (the grammar still round-tripsp.Met1Valext-10).Ter110GlnextTer17shape is kept. A multi-residueaatermtail sequence is reduced to its spec slots: the first residue becomes the aa replacing the stop codon (only whenaltis unset) and the last residue becomes the new terminator. Single-residueaatermvalues (everything the parser itself produces, and whataltseq_to_hgvspconstructs) format exactly as before.Verification
test_AAExt_issue804intests/test_hgvs_edit.pyuses the issue's exact examples plus the spec's expected forms. It failed on pristine upstream code (Met1extLeuGlyMet-2,Ter10extLeuGlyTer2) and passes with the fix (Met1ext-2,Ter10LeuextTer2).test_hgvs_edit,test_hgvs_posedit,test_hgvs_grammar,test_hgvs_grammar_equivalence,test_hgvs_grammar_full,test_hgvs_parser,test_hgvs_location,test_hgvs_sequencevariant(63 passed, 27 skipped; skips are pre-existing data-hungry/network tests).p.Ter314Trpext*45->p.Ter314TrpextTer45,p.Met1ext-12unchanged, legacyp.Met1extMet-5now normalizes to the spec formp.Met1ext-5,p.Ter327ArgextTer?unchanged. Thep_term_asteriskconfig path (Leuext*2) and uncertain wrapping ((ext-2)) also verified.What I could not verify
AAExtformatting that depends on live data (e.g.variantmapper,validator) was not exercised.