Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/null deletion edge case #34

Merged
merged 5 commits into from
Jun 28, 2024
Merged

Conversation

JeremyWesthead
Copy link
Collaborator

Give None for gene position in cases where the start of a deletion in a revcomp gene goes past the start of the gene's promoter

@mcolpus
Copy link
Contributor

mcolpus commented Jun 27, 2024

What would be the impact of this for the katG deletion?

][-1]
> nc_num
):
return None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it easy to add a test case for this just to make sure there aren't any off by one errors, or something like that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not particularly easy to add a test case for this - it'd either have to be a new dummy genome or a contrived TB test case (slow). However, it shouldn't be off by one as > should be checking that the calculated nc_num is less than the minimum gene position

@JeremyWesthead
Copy link
Collaborator Author

What would be the impact of this for the katG deletion?

This should result in 0 changes to the prediction, but should change the variants block

        "nucleotide_index": 2156034,
        "gene_name": "katG",
        "gene_position": null,
        "codon_idx": null,

This gives a null value for gene_position rather than a misleading position which doesn't exist

@JeremyWesthead JeremyWesthead merged commit 4e1bd25 into master Jun 28, 2024
2 checks passed
@JeremyWesthead JeremyWesthead deleted the fix/null-deletion-edge-case branch June 28, 2024 12:39
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