Fix ins or dups where splice region is preserved #719

b0d0nne11 · 2024-01-23T19:25:46Z

Fixes #714.

Fixes ins or dup variants spanning the intron/exon or exon/intron boundary where the splice site & region remain completely intact.

b0d0nne11 · 2024-02-07T14:15:09Z

We found some examples of duplications where the original logic here didn't shift the variant far enough to to get the expected result. It turned out that for these variants it's not possible to write the shifted version as a duplication since in a duplication the alt will always follow the ref. I've added some logic to rewrite these shifted variants as insertions before attempting to map them back to var_ps and added tests to include these cases.

gostachowiak · 2024-02-07T14:57:44Z

High-level explanation of this pull request:

HGVS nomenclature has the 3' shifting rule. so all cdots and pdots are shifted to the right
However, 3' shifting is arbitrary and a necessary evil for nomenclature purposes. But biology doesn't care about the 3' shifting rule
Consider an example: positive strand gene, first 8 bases of the intron is duplicated
cdot would be +1_+8dup
currently no pdot would be calculated because both positions have an offset
but now consider the biology-- after the duplication, there are 2 splice sites on the left side of the intron. Which is more likely to be used for splicing?
We obviously can't know for sure, but it seems to me that the most logical assumption is that it will use the "inner" splice site for splicing, leaving the extra inserted material within the coding sequence, resulting in a frameshift pdot. Why is this the best assumption?
- When that splice site is used, the entire intronic sequence is totally intact
- Seems "safer" to calculate a pdot for this to rescue the variant-- otherwise downstream applications will most likely be filtering out this variant because it's an insertion after the 8th position in the intron
To handle this type of situation in the most general way possible, this is the approach:
- calculate pdot the normal way.
- if empty, shift the cdot in the REVERSE direction, and calculate the pdot again. if you get a result, use it
- basically, if you can get a pdot with either forward or reverse shifting, that means the entire intron is intact and we should bring in the pdot
note that the 3' shifting rule is still respected both for the cdot and pdot nomenclature
- reverse shifting is only used as a tool when calculating pdot from cdot, which does not violate HGVS nomenclature

gostachowiak · 2024-02-08T12:16:05Z

I also wanted to mention that we discovered this because some fraction of FLT3 ITDs currently get missed when using the hgvs package (including one added to the unit tests). So it is a high impact issue.

github-actions · 2024-03-11T01:49:37Z

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

github-actions · 2024-04-11T01:52:11Z

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

b0d0nne11 · 2024-04-12T14:07:52Z

@reece or @ahwagner can you remove the stale label here? We would still like to get this merged is possible. Thanks!

b0d0nne11 · 2024-05-24T16:24:12Z

We found a case where trying to map the shifted variant causes an HGVSInvalidVariantError. I've added logic to handle this and a test case. The variant is NM_182758.2:c.2953-31_2953-26dup. As part of the shifting procedure, mapping this to the g type yielded an unexpected transformation to NC_000015.9:g.53815545_53815550delinsC that caused problems with later steps. I'm simply handling the error here since we don't want to consider invalid variants.

b0d0nne11 · 2024-05-24T16:24:33Z

Also rebased on main

b0d0nne11 · 2024-06-04T18:17:31Z

Rebased on main

b0d0nne11 · 2024-09-14T23:13:11Z

Made a few non-functional edits:

Rebased on main
Moved tests to an isolated test file
Squashed some commits

github-actions · 2025-01-01T02:23:39Z

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

b0d0nne11 · 2025-09-04T19:26:11Z

Hi @korikuzma @ahwagner et all, I've removed the flags on sequence variants that I added previously as we found a way to handle this in the logic on our end. I've also rebased on main so clear up any conflicts. I think this is ready for a review when you guys have time. Thanks!

github-actions · 2025-09-12T02:19:12Z

This issue is stale because it has been open 180 days with no activity.
Remove stale label or comment or this will be closed in 30 days.

b0d0nne11 · 2025-10-01T19:06:35Z

Hi again @korikuzma @ahwagner @reece, can I get the stale label removed here? We would still like to get this merged if possible. Also if you have any feedback that would be great. Thanks!

reece · 2025-10-01T20:44:37Z

Hi @b0d0nne11 - I removed the stale label.

I noticed that there are conflicts with main. We'll need to resolve those in order to review.

I'm going to be without a computer for a week (😱) but I'll take a look when I return.

b0d0nne11 · 2025-10-03T20:35:25Z

Hey @reece, thanks! I rebased on master and took care of the merge conflicts. I'll also be out next week so no rush. Whenever you have time would be great.

github-actions · 2025-10-11T02:17:58Z

This issue is stale because it has been open 180 days with no activity.
Remove stale label or comment or this will be closed in 30 days.

b0d0nne11 · 2025-10-13T13:42:42Z

Hi @reece, looks like this got marked stale again. Can you take that off?

github-actions · 2025-10-21T02:37:04Z

This issue is stale because it has been open 180 days with no activity.
Remove stale label or comment or this will be closed in 30 days.

reece

@b0d0nne11- This is a great contribution! Thank you! I especially like the shift functions to explore alternative representations. And I appreciate your care in enabling users to opt into new behavior without impinging on existing functionality.

We're working toward a 2.0 release in December. I'd love to include this feature.

reece · 2025-10-27T16:00:19Z

src/hgvs/_data/defaults.ini

 prevalidation_level = EXTRINSIC
 replace_reference = True
 ins_at_boundary_is_intronic = True
+shift_over_boundary = False


@b0d0nne11: Would you please add a short comment above each of these describing what they do? Also note that False + DEFAULT correspond to hgvs 1.x code behavior and the team's interpretation of HGVS Nomenclature.

reece · 2025-10-27T18:06:51Z

src/hgvs/utils/altseqbuilder.py

        return alt_data

-    def _get_variant_region(self):
+    def get_variant_region(self):


The complexity of these cases warrants explicit tests. Would you please implement those? I think that it might be enough to roll these cases into the cases block of test_714.py with an explicit call to get_variant_region().

reece · 2025-10-27T18:24:38Z

tests/data/gcp/real.tsv

 ID00049	NC_000010.10:g.89692921dupA	NM_000314.4:c.405dupA	NP_000305.3:p.(Cys136Metfs*44)
 ID00050	NC_000010.10:g.89692923_89692939delGTGCATATTTATTACAT	NM_000314.4:c.407_423delGTGCATATTTATTACAT	NP_000305.3:p.(Cys136Serfs*38)
 ID00051	NC_000010.10:g.89712015C>A	NM_000314.4:c.633C>A	NP_000305.3:p.(Cys211*)
-ID00052	NC_000010.10:g.89685314dupT	NM_000314.4:c.209dupT	NP_000305.3:p.?


Please comment why these tests were removed. It scares me to remove tests because it implies that the tests were wrong and therefore that the code was passing something that it should have. These particular tests were contributed by some extremely experienced clinical geneticists at Invitae (in ~2015).

To be clear, we should remove them if they're wrong. I just want to know that rationale explicitly. And, while I can see the two deleted tests might be questionable, the rationale that I would use would implicate most or all of these p.? tests, not just these two.

reece · 2025-10-27T18:27:53Z

src/hgvs/variantmapper.py

        var.gene = symbol
        return var

+    def _var_c_shifts(self, var_c, alt_ac, alt_aln_method):


Please add tests for the _var_c_shifts and _var_g_shifts methods.

andreasprlic · 2025-10-29T04:35:03Z

src/hgvs/variantmapper.py

+        duplications as insertions so that the variant is shifted farther
+        than would normally be possible using the HGVS notation."""
+        var_g = copy.deepcopy(var_g)
+        normalizer = hgvs.normalizer.Normalizer(


rather than creating this here on every call of _var_g_shift_with_rewrite, should we move this (perhaps even as an optional argument) into the VariantMapper.__init__()?

andreasprlic · 2025-10-29T04:35:58Z

src/hgvs/variantmapper.py

+        normalizer = hgvs.normalizer.Normalizer(
+            self.hdp, alt_aln_method=alt_aln_method, validate=False, shuffle_direction=shuffle_direction
+        )
+        var_g = normalizer.normalize(var_g)


I did not test this, but I suspect that calling normalize() is slow. What would happen if we would not call the normalizer here?

andreasprlic · 2025-10-29T04:39:13Z

Overall I am very happy with this PR, and I would like to approve it, once Reece's comments have been addressed (mostly about testing). This is an improvement that shows great attention to detail and that solves an important edge case in a way that can be configured (and the default is compliant with HGVS nomenclature). Thanks for putting in all this effort! This is greatly appreciated.

github-actions · 2025-11-06T02:43:41Z

This issue is stale because it has been open 180 days with no activity.
Remove stale label or comment or this will be closed in 30 days.

b0d0nne11 requested a review from a team as a code owner January 23, 2024 19:25

b0d0nne11 force-pushed the 714-splice-region-preserved branch from 0530e68 to 7f18cfd Compare January 23, 2024 19:27

b0d0nne11 force-pushed the 714-splice-region-preserved branch 2 times, most recently from f2f89da to 08cea20 Compare February 8, 2024 15:34

github-actions bot added the stale Issue is stale and subject to automatic closing label Mar 11, 2024

ahwagner removed the stale Issue is stale and subject to automatic closing label Mar 11, 2024

github-actions bot added the stale Issue is stale and subject to automatic closing label Apr 11, 2024

jsstevenson added keep alive exempt issue from staleness checks and removed stale Issue is stale and subject to automatic closing labels Apr 12, 2024

b0d0nne11 force-pushed the 714-splice-region-preserved branch from 08cea20 to 13f7e36 Compare May 24, 2024 16:24

b0d0nne11 force-pushed the 714-splice-region-preserved branch from 13f7e36 to efdf6d0 Compare June 4, 2024 18:17

holtgrewe mentioned this pull request Jul 24, 2024

Port over biocommon/hgvs#719 (Fix ins or dups where splice region is preserved) varfish-org/hgvs-rs#194

Open

ahwagner self-requested a review August 26, 2024 16:44

ahwagner assigned ahwagner and unassigned ahwagner Aug 26, 2024

b0d0nne11 force-pushed the 714-splice-region-preserved branch from efdf6d0 to eaf2ef6 Compare September 14, 2024 23:11

b0d0nne11 force-pushed the 714-splice-region-preserved branch 2 times, most recently from fd853be to 10613ad Compare September 18, 2024 20:34

korikuzma removed the keep alive exempt issue from staleness checks label Dec 1, 2024

b0d0nne11 force-pushed the 714-splice-region-preserved branch from 7a6c23f to 236cdd0 Compare September 4, 2025 19:22

b0d0nne11 temporarily deployed to pypi September 4, 2025 19:23 — with GitHub Actions Inactive

github-actions bot added the stale Issue is stale and subject to automatic closing label Sep 12, 2025

reece removed keep alive exempt issue from staleness checks stale Issue is stale and subject to automatic closing labels Oct 1, 2025

b0d0nne11 added 4 commits October 3, 2025 14:56

Fix ins/dups where splice region is preserved

efb64a5

Make var_c shifts easier to get from the assembly mapper

955a980

Remove new flags on sequence variants

8b87dfa

Update test cache

6303c8f

b0d0nne11 force-pushed the 714-splice-region-preserved branch from 236cdd0 to 6303c8f Compare October 3, 2025 20:26

b0d0nne11 temporarily deployed to pypi October 3, 2025 20:30 — with GitHub Actions Inactive

github-actions bot added the stale Issue is stale and subject to automatic closing label Oct 11, 2025

reece removed the stale Issue is stale and subject to automatic closing label Oct 14, 2025

github-actions bot added the stale Issue is stale and subject to automatic closing label Oct 21, 2025

reece requested changes Oct 27, 2025

View reviewed changes

reece requested a review from andreasprlic October 27, 2025 18:31

github-actions bot removed the stale Issue is stale and subject to automatic closing label Oct 28, 2025

andreasprlic reviewed Oct 29, 2025

View reviewed changes

github-actions bot added the stale Issue is stale and subject to automatic closing label Nov 6, 2025

Fix ins or dups where splice region is preserved #719

Are you sure you want to change the base?

Fix ins or dups where splice region is preserved #719

Uh oh!

Conversation

b0d0nne11 commented Jan 23, 2024

Uh oh!

b0d0nne11 commented Feb 7, 2024

Uh oh!

gostachowiak commented Feb 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gostachowiak commented Feb 8, 2024

Uh oh!

github-actions bot commented Mar 11, 2024

Uh oh!

github-actions bot commented Apr 11, 2024

Uh oh!

b0d0nne11 commented Apr 12, 2024

Uh oh!

b0d0nne11 commented May 24, 2024

Uh oh!

b0d0nne11 commented May 24, 2024

Uh oh!

b0d0nne11 commented Jun 4, 2024

Uh oh!

b0d0nne11 commented Sep 14, 2024

Uh oh!

github-actions bot commented Jan 1, 2025

Uh oh!

b0d0nne11 commented Sep 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Sep 12, 2025

Uh oh!

b0d0nne11 commented Oct 1, 2025

Uh oh!

reece commented Oct 1, 2025

Uh oh!

b0d0nne11 commented Oct 3, 2025

Uh oh!

github-actions bot commented Oct 11, 2025

Uh oh!

b0d0nne11 commented Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Oct 21, 2025

Uh oh!

reece left a comment

Choose a reason for hiding this comment

Uh oh!

reece Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

reece Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

reece Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

reece Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

andreasprlic Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

andreasprlic Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

andreasprlic commented Oct 29, 2025

Uh oh!

github-actions bot commented Nov 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

gostachowiak commented Feb 7, 2024 •

edited

Loading

b0d0nne11 commented Sep 4, 2025 •

edited

Loading

b0d0nne11 commented Oct 13, 2025 •

edited

Loading