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 export of strandedness #1211

Merged
merged 4 commits into from
May 25, 2023
Merged

fix export of strandedness #1211

merged 4 commits into from
May 25, 2023

Conversation

rneher
Copy link
Member

@rneher rneher commented May 12, 2023

The strand of translated features is represented in the exported json via + or -. But this export didn't work as intended because augur translate wrongly assumed feature_location.strand is boolean when instead it is +1 or -1. This commit additionally accounts for unknown (0 -> ?) or unspecified strandedness (None -> None).

The line changed was suggested by @tsibley in this comment

superseeds PR 966

@rneher
Copy link
Member Author

rneher commented May 12, 2023

It looks as if our tests were happily asserting that the wrong annotation is correct.

$ python3 "../../scripts/diff_jsons.py" translate/data/tb/aa_muts.json $TMP/aa_muts.json
-  {}
+  {'values_changed': {"root['annotations']['*MtCM']['strand']": {'new_value': '-', 'old_value': '+'}, "root['annotations']['PE_PGRS39']['strand']": {'new_value': '-', 'old_value': '+'}, "root['annotations']['PPE13']['strand']": {'new_value': '-', 'old_value': '+'}, "root['annotations']['Rv0010c']['strand']": {'new_value': '-', 'old_value': '+'}, "root['annotations']['Rv0011c']['strand']": {'new_value': '-', 'old_value': '+'}, "root['annotations']['Rv0039c']['strand']": {'new_value': '-', 'old_value': '+'}, "root['annotations']['Rv0323c']['strand']": {'new_value': '-', 'old_value': '+'}, "root['annotations']
[...]

The gene *MtCM for example should be on the negative strand:

MTB_anc	RefSeq	gene	2134273	2134872	.	-	.	ID=gene1921;Dbxref=GeneID:885772;Name=*MtCM_Rv1885c;gbkey=Gene;gene=*MtCM;

@rneher rneher requested a review from huddlej May 13, 2023 09:20
Copy link
Contributor

@huddlej huddlej left a comment

Choose a reason for hiding this comment

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

Beyond the merge conflict, this looks good to me!

rneher and others added 4 commits May 25, 2023 22:12
The strand of translated features is represented in the exported
json via `+` or `-`. But this export didn't work as intended because
augur translate wrongly assumed `feature_location.strand` is boolean
when instead it is +1 or -1. This commit additionally accounts for
unknown (0 -> ?) or unspecified strandedness (None -> None).
Testing if this fixes the test by exporting [-1,1] strand values instead
of [0,1] strand values. Might require more discussion if this is the right
direction since this may cause previously generated v1 exported data to fail
validation due to [0,1] values.
@rneher rneher force-pushed the fix/translate-strand-annotation branch from 83389c1 to 4f4075f Compare May 25, 2023 20:13
@rneher rneher merged commit b61e3e7 into master May 25, 2023
@rneher rneher deleted the fix/translate-strand-annotation branch May 25, 2023 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

3 participants