Clade Specification by AA and Nuc #206
Conversation
augur/clades.py
Outdated
| ''' | ||
| Determines whether a node contains all mutations that define a clade | ||
| ''' | ||
| isClade = False |
There was a problem hiding this comment.
For consistency, isClade should be snakecased like is_clade.
augur/clades.py
Outdated
| isClade = False | ||
| for gene, muts in clade_mutations.items(): | ||
| if (gene in node_muts and node_muts[gene] != []): | ||
| prs_muts = [(int(tu[1:-1]), tu[-1]) for tu in node_muts[gene]] #get mutations in right format |
There was a problem hiding this comment.
It is hard to tell what is happening here based on the variable names and operations alone. Can you rename these variables (especially prs_muts) to be more descriptive and include an inline comment above this line with an example of what tu looks like?
There was a problem hiding this comment.
the comparison with empty list is not ideal -- I am always worried it might be comparing pointers or similar. The != [] isn't actually needed, empty lists evaluate to false.
augur/translate.py
Outdated
| @@ -170,6 +170,17 @@ def construct_mut(start, pos, end): | |||
| def assign_aa_vcf(tree, translations): | |||
| aa_muts = {} | |||
|
|
|||
| #get mutations on the root | |||
| root = tree.get_nonterminals()[0] | |||
There was a problem hiding this comment.
You can use tree.root to get the root node.
augur/clades.py
Outdated
|
|
||
| def read_in_clade_definitions(clade_file): | ||
| ''' | ||
| Reads in tab-seperated file that defines clades by amino-acid. |
There was a problem hiding this comment.
This docstring made me think clades couldn't be defined by nucleotides, but seeing the real TB clade definitions clarified this. Could you update the docstring comment and example to mention nucleotides, too?
augur/clades.py
Outdated
| isClade = False | ||
| for gene, muts in clade_mutations.items(): | ||
| if (gene in node_muts and node_muts[gene] != []): | ||
| prs_muts = [(int(tu[1:-1]), tu[-1]) for tu in node_muts[gene]] #get mutations in right format |
There was a problem hiding this comment.
the comparison with empty list is not ideal -- I am always worried it might be comparing pointers or similar. The != [] isn't actually needed, empty lists evaluate to false.
augur/clades.py
Outdated
| node_data = read_node_data(args.mutations, args.tree) | ||
| if node_data is None: | ||
| print("ERROR: could not read node data (incl sequences)") | ||
| return -1 |
There was a problem hiding this comment.
standard error code would be +1
http://tldp.org/LDP/abs/html/exitcodes.html
There was a problem hiding this comment.
I copied this from translate, so I'll change there as well
|
Thank you both - happy to make these changes. |
|
If all is ok, can we merge this? It has changes to the TB run, so would be easier to merge before I pull TB out to a separate repo. |
This PR should allow specification of clades by nucleotide and/or AA, as was previously done (hopefully) in old augur.
I based this from what I found in the flu builds, but it would be good for someone to check this does all it needs to - particularly for flu! I preserved the method of specifying the mutations by 'gene position alt' to make moving these definitions easier.
One change is that
translatenow writes out mutations on the root node. For Fasta-input this is empty, but for VCF input it will contain mutations from the reference. This was mostly to makeutilsread_node_datawork - otherwise the tree andaa_mutsJSON don't have the same number of nodes. But it has a nice side-effect of being able to see AA (and in particular DRM) mutations that apply to the whole dataset with VCF input - matching what's often shown in TB papers, for example.All the rest slots nicely into existing
auspicefunctionality (clade_annotationsets node labels,clade_membershipcan be a colorBy), and if 633 is merged inauspice, the clades specified here can allow zooming on the tree by clade via URL.I've changed the TB build so that it now uses this rather than the 'trait' 'cluster', and I've added it to the TB test as well - though it doesn't work perfectly as a node cannot be a clade, so it looks a bit off for a couple of nodes.
Clades in TB (rather than cluster, as previously):
Clade labels persist even when you change the colorBy.