Conversation
Remove cli boolean flag that allowed for inclusion of v2 in addition to v1.3
…variant Currently, the only case where this will occur is when the variant falls outside of the region of the target sequence that aligns to the genomic reference sequence. Post-map protein variants are still required to output the corresponding pre-map protein variant, because we don't expect a protein variant to fall outside the region of the target sequence that aligns to the reference transcript, although this assumption should be evaluated as a separate issue.
|
Marking this as draft because this branch is based on the previous PR #15, which I'll want to merge into dev before this one. |
| if accession is None: | ||
| raise ValueError |
There was a problem hiding this comment.
Could we add a message to these raised ValueErrors? Even if we don't expect them to occur the message offers the same thing as a comment but with useful output if it does ever end up popping up for a user.
| if tx_results is None: | ||
| raise ValueError # impossible by definition |
There was a problem hiding this comment.
Could we add a message to these raised ValueErrors? Even if we don't expect them to occur the message offers the same thing as a comment but with useful output if it does ever end up popping up for a user.
Same as above
| for allele in post_mapped.members: | ||
| loc = allele.location | ||
| sequence_id = f"ga4gh:{loc.sequenceReference.refgetAccession}" | ||
| ref = sr.get_sequence(sequence_id, loc.start, loc.end) # TODO type issues?? |
There was a problem hiding this comment.
Is this TODO still relevant?
| False, | ||
| ) | ||
| else: | ||
| # TODO add error to MappedScore |
There was a problem hiding this comment.
Commenting so this TODO isn't lost, but I know this is still a draft PR.
| # TODO add error to MappedScore | ||
| post_mapped_genomic = None | ||
|
|
||
| if pre_mapped_genomic: |
There was a problem hiding this comment.
Similar to a few comments above. We know whether pre_mapped_genomic is defined on line 394, so we should be able to check if it is None and return immediately. Here I'm less inclined to leave as is for readability reasons because of the addition of the if/else block above this one.
So our flow could look something like:
pre_mapped_genomic = _construct_vrs_allele
if pre_mapped_genomic is None:
return None
try to do post mapped genomic
return the mapped score
| ) | ||
|
|
||
| if pre_mapped_protein and post_mapped_protein: | ||
| if pre_mapped_protein: |
There was a problem hiding this comment.
We know this on 344, so we could just check pre_mapped_protein right after constructing it and return None without attempting to build the post mapped protein that we won't need. (I'm sort of sympathetic to the argument the code looks nicer the way it is though).
pre_mapped_protein = _construct_vrs_allele
if pre_mapped_protein is None:
return None
post_mapped_protein = _construct_vrs_allele
return mapped score
sallybg
force-pushed
the
improve-vrs-2-output
branch
from
6a499fc to
8b2b75e
Compare
|
Closing in favor of #20. |
2 participants
Allow pre-map genomic variant without corresponding post-map genomic variant.
Currently, the only case where this will occur is when the variant falls
outside of the region of the target sequence that aligns to the genomic
reference sequence. Post-map protein variants are still required to
output the corresponding pre-map protein variant, because we don't
expect a protein variant to fall outside the region of the target
sequence that aligns to the reference transcript, although
this assumption should be evaluated as a separate issue.