Skip to content

Vrs output#855

Draft
John-F-Wagstaff wants to merge 3 commits into
developfrom
vrs_output
Draft

Vrs output#855
John-F-Wagstaff wants to merge 3 commits into
developfrom
vrs_output

Conversation

@John-F-Wagstaff

Copy link
Copy Markdown
Collaborator

Initial version of the VRS output code for VV, valid for VRS 2.1.

@Peter-J-Freeman Please look through the output structure with an eye to whether we want to change it before it goes in, as well as checking over the code.

The variant object set up has accumulated a number of now unneeded
global variables for hgvs data, several of which where only needed prior
to converting the quibble to a hgvs object. These make the code more
complex to understand and manage and interfered with finding the correct
data for VRS input, so purge them while the understanding is still fresh
from untangling the VRS inputs.
We used to store a number of hgvs descriptions for variation as mapped
to specific targets e.g. genome, RSG, protein.  Now that we need these
as objects, to make VRS output, switch this to happen only at the last
output processing step.
@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.68878% with 73 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.67%. Comparing base (9a412ff) to head (52f211b).

Files with missing lines Patch % Lines
VariantValidator/modules/use_checking.py 75.86% 35 Missing ⚠️
VariantValidator/modules/mappers.py 86.88% 8 Missing ⚠️
VariantValidator/modules/vrs_utils.py 97.82% 8 Missing ⚠️
VariantValidator/modules/valoutput.py 82.05% 7 Missing ⚠️
VariantValidator/modules/complex_descriptions.py 85.29% 5 Missing ⚠️
VariantValidator/modules/format_converters.py 81.81% 4 Missing ⚠️
VariantValidator/modules/hgvs_utils.py 62.50% 3 Missing ⚠️
VariantValidator/modules/vvMixinCore.py 95.00% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #855      +/-   ##
===========================================
+ Coverage    77.11%   77.67%   +0.55%     
===========================================
  Files           37       38       +1     
  Lines        13071    13488     +417     
===========================================
+ Hits         10080    10477     +397     
- Misses        2991     3011      +20     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@John-F-Wagstaff

Copy link
Copy Markdown
Collaborator Author

OK so the test failure is for ccds_ids

I got ['CCDS14231', 'CCDS14232', 'CCDS14234', 'CCDS14230', 'CCDS55394', 'CCDS94585', 'CCDS14233', 'CCDS48091', 'CCDS94586', 'CCDS14229', 'CCDS55395'] the test machine gets [] for variant input of 'NC_000023.10:g.33229673A>T' via 'NM_000109.3:c.7+127703T>A' should I just purge the check or is this something we want to fix?

@Peter-J-Freeman

Copy link
Copy Markdown
Collaborator

Hi John,

I will look properly on Monday. Bit busy today

for the failing test, it is not a critical assert, so perhaps just amend the test so it passes???

@John-F-Wagstaff

Copy link
Copy Markdown
Collaborator Author

OK, I will get it fixed and pushed up before then.

It is not a super hurry, I just finally wanted to get it completed and submitted. This is high priority, but more urgent supposedly shorter jobs have kept on turning up, often right as I was getting into the swing of things, and breaking my metaphorical stride. As a result this code has been hanging over me as a "should complete ASAP" for far too long. It is a relief to finally sign off on it as done.

@Peter-J-Freeman

Copy link
Copy Markdown
Collaborator

Dont go to a huge amount of effort. Just remove the offending assert if possible and commit and push. No need to do full pulling down of the commit etc. Or just remove the failing test unless it adds very needed fucntionality

@Peter-J-Freeman

Copy link
Copy Markdown
Collaborator

We are gonna have to plan a merge strategy too. As I want to pull this in.

#854

Lots of bugs identified and squashed

@John-F-Wagstaff

Copy link
Copy Markdown
Collaborator Author

For merging strategy, I suggest that we merge #854 first then you tell me. I will re-base this code on top of that. Only after that will we finalise this.

You have added a lot of tests to the main VV code base so any issues caused by the first two patches in this set are best examined by moving this "after" that set in git history terms.

edit: It also might settle some of the intermittent DB connection issues, which just affected the tests.

@John-F-Wagstaff John-F-Wagstaff marked this pull request as draft July 6, 2026 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants