Vrs output#855
Conversation
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 Report❌ Patch coverage is 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. 🚀 New features to boost your workflow:
|
|
OK so the test failure is for ccds_ids I got |
|
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??? |
|
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. |
|
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 |
|
We are gonna have to plan a merge strategy too. As I want to pull this in. Lots of bugs identified and squashed |
5d8d65e to
52f211b
Compare
|
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. |
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.