fix: handle in-frame complex variants in _get_within_cds_effect()

[sallykinyua]

Fixes #1132

Implements the previously unhandled else branch in
_get_within_cds_effect() for in-frame complex variants — cases where
multiple ref bases are replaced by a different number of alt bases with
a net change divisible by 3 (reading frame preserved).

Changes:

• Applies the same is_codon_changed logic already used for simple
  insertions and deletions, assigning:
  • CODON_CHANGE_PLUS_CODON_INSERTION / CODON_INSERTION for net insertions
  • CODON_CHANGE_PLUS_CODON_DELETION / CODON_DELETION for net deletions
• Adds the first unit tests for veff.py (41 tests, all passing)

No new effect types introduced. Labels are consistent with SnpEff
terminology already used elsewhere in the function.

[jonbrenas]

Thank you @sallykinyua. Your solution works, mostly.

The first issue is that it makes a large bit of code redundant (the cases where either len(alt) == 1 or len(ref) == 1 are covered by your example).
The second issue is bigger: the problem isn't correctly framed. In the case where either len(alt) == 1 or len(ref) == 1, it makes sense to decide which codon is the one that can be modified but if ref is 2 codons and alt is 3 codons whether the first one or the second one of ref gets modified should not make a difference. Because there are no actual use cases, fixing the TODO right now makes little to no sense, and I would thus argue that there is no good reason to do it.

[sallykinyua]

@jonbrenas I fully acknowledge your input and actually understand on the first and second issue. However,would you be open to a minimal PR that replaces
the TODO string with a NotImplementedError with an explicit message?
That at least fails loudly instead of silently returning garbage,
and leaves the door open for a correct implementation when a real
use case appears.

[jonbrenas]

Hi @sallykinyua.

However, would you be open to a minimal PR that replaces the TODO string with a NotImplementedError with an explicit message?

Raising an error would make things worse. This code is currently never reached. If a user manages to do something weird enough that they get to this code, an error that the user can't do anything about will not be more helpful than a return that says that this hasn't been done yet.

leaves the door open for a correct implementation when a real use case appears.

That's the thing, though, there isn't a real use case (and there shouldn't be). Actually, in my opinion, the best solution would be to ensure that 'len(ref) == len(alt) == 1' is always true, but I don't want that to be done right now (i.e., not until we decide that it is really what we want to do).