Consider merging VF into the VV Repo and updating the pyproject.toml to install both packages

[Peter-J-Freeman]

Is your feature request related to a problem? Please describe.
The VF tests live in the VV repo. Consequently, VF is included in CI and coverage but never gets the coverage metrics applied. It also means we are maintaing 2 Repos which makes management more difficult

Describe the solution you'd like
Pull the VF code base into the VV Repo
Update pyproject.toml to include e.g.

[tool.setuptools.packages.find]
include = [
    "VariantValidator*",
    "VariantFormatter*"
]

then a single install with pip install . will install both packages, but they will remain seprate entities so will not break the existing workflows

Describe alternatives you've considered
Keep 2 repos

Additional context
Add any other context or screenshots about the feature request here.

[Peter-J-Freeman]

@John-F-Wagstaff, thoughts please.

An argument could also be made to additionally pull vv_hgvs into the VV Repo as well, then strip out all the code we don't use. It seems like we are continuing to drift from the original hgvs.py. This way we could clean up vv_hgvs, and include the tests we want into the VV repo. Then split the tests into

tests - VV_tests
- VF_tests

and if we go this way
- vv_hgvs_tests.

The other advantage is that it would remove this section of the current VV build

    "vvhgvs@git+https://github.com/openvar/vv_hgvs@master",
    "VariantFormatter@git+https://github.com/openvar/variantFormatter@master"

which is our key area for loss of reproducibility. Pulling everything into 1 repo would all tools will share the same version IDs as they pull the version from the coimmit directly. This would mean VV version == VF version == VV_hgvs version meaning complete reproducibility. I think it will also reduce technical debt

[John-F-Wagstaff]

An argument could also be made to additionally pull vv_hgvs into the VV Repo as well

Although we might benefit, testing wise, from moving vv_hgvs into the same repo excessive integration between to many components with too high a complexity, both individually and in their interactions, is one of the key structural problems we face. This would make things worse.

If we did want to make a clean break form hgvs we would probably be better doing the fixes in the separate repo, so far that would be:

1. putting in my more complete fixes for the second half of the transcript<->genomic mappings in
2. rolling in some of the extra fixes we currently do inside VV
3. remaking and upgrading the internal vv_hgvs tests to match our current style
4. cutting out code that we won't be using in the future (the second mapper)

This would allow us to simplify our relationships between VV and vv_hgvs doing both less back and forth and less weird use of vv_hgvs components inside VV, without making the whole project more of a tarball. Unfortunately all of the gains here require significant work.

[Peter-J-Freeman]

So you are not opposed to merging VF into VV. For me, this seems sensible. Also, easier to maintain.

For vv_hgvs, potentially plan a totally new clean version with all the changes you would want to make and ideally clean up a lot to remove weight and hopefully speed improvements? But, it will be subject to time and funding of course.

We have been waiting to see if we can re-integrate for years now, and what I mostly see from hgvs updates are things that address stuff we fixed years ago. the only thing they may get better is gap mapping, but I know you aee hoping to make improvements there too

[John-F-Wagstaff]

The VV VF merger is more possibly helpful but would require significant thinking, particularly since the complex interactions between the two are not ideal.

We probably ideally want to split it into VariantValidatorCore, VariantValidator (classic) and VariantFormater, where both VV and VF depend on VariantValidatorCore but not each other and the VariantValidatorCore system is structured in such a way as to make this sensible. This of course would be a lot of work, and it would also make splitting the system into 3 packages sensible, or at least it might.

We probably want to do the current variable cleanup, then do some more deep diving into clearing up things that should just depend on the Variant (or perhaps the mapping object I added to keep cached mappings so as to avoid forcing VF to do things with fake VV objects) and then look at the result and have a long chat on the intended final architecture before we come to a final decision on this.

[Peter-J-Freeman]

Good plan. We can put this on the back burner, but I would very much like to do it.

In terms of use, it changes nothing you would still use import VariantValidator import VariantFormatter

A chane in wither tool would cause an overall version update to both tools, which should have the same version. For me, this is much better for reproducibity and maintenence.

[ifokkema]

FWIW:

Although we might benefit, testing wise, from moving vv_hgvs into the same repo excessive integration between to many components with too high a complexity, both individually and in their interactions, is one of the key structural problems we face. This would make things worse.

I have the LOVD/HGVS library in a repo separated from the main LOVD3 repo. This is mainly because this library is in use by other projects as well. The LOVD/VV library is included in the LOVD/HGVS repo because LOVD/VV doesn't do anything without LOVD/HGVS. However, keeping this all separated from the LOVD3 repo allows me to use them separately and test them separately. Whenever I update the LOVD/HGVS library and the local tests succeed, I should update the LOVD/HGVS library in other repos like the LOVD3 repo or the api.lovd.nl repo, after which I need to run those tests also. Sometimes that highlights additional problems, but this means that the LOVD/HGVS tests aren't exhaustive, and I need to update them. Anyway, I agree that keeping vv_hgvs separate would be better, especially because it's an old fork.

So you are not opposed to merging VF into VV. For me, this seems sensible. Also, easier to maintain.

In my limited understanding, there is a lot of VV and VF code that is similar or the same, since much of the functionality is similar. That would justify merging, as it would (hopefully) be simpler to maintain and test the code.

[Peter-J-Freeman]

Yeah,

To me it boils down to VV must import VF and VF must import VV so keep them together as they are no longer separateable entities

[John-F-Wagstaff]

VV must import VF and VF must import VV

That is true but almost all of the time it is VF depending on VV, the other way around is rare in fact the only usage point of VV in VF is rna_formatter.

We would probably be better served by making the dependency one way in either case this kind of cyclical dependency is headache inducing and drives maintenance issues among other problems.

It should only be a small change to do it too, at least after I have sorted out the global mapper setting issues, maybe ~3-5 lines of code added to VV.

@Peter-J-Freeman Are there any other places that it gets called that I have missed?

[Peter-J-Freeman]

I dont think so. The reason I included VF for this task is that it does it fast without having to write a load more code into VV.

It may be a simpler fix now than it was.

Will wait till the global mappers etc are in place too.

[John-F-Wagstaff]

It looked like after the mappers are sorted out the function used (hgvs_transcript2hgvs_protein) just does

    # Create dictionary to store the information    
    re_to_p = False
    hgvs_transcript_to_hgvs_protein = vfo.myc_to_p(hgvs_transcript, evm, re_to_p, rhn)
    hgvs_transcript_to_hgvs_protein = hgvs_transcript_to_hgvs_protein['hgvs_protein']
    return hgvs_transcript_to_hgvs_protein

excluding the return statement that is 3 statements executed, if more than that in lines. It was more before, due to initialisation of mappers and mapper selection etc, but after handling the global mapper setup this will be done already (it needs to happen before here in the logic to make the tandem repeats code work).