-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add recombinant clade definition #69
Conversation
Our automated model runs failed on 2023-10-30¹ with AssertionError raised by `modify-lineage-colours-and-order.py` ``` AssertionError: Missing definitions for the following clades: recombinant ``` ¹ https://bedfordlab.slack.com/archives/C03BY4MPNCS/p1698733327193829
color: '#515151' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jameshadfield, I'm not sure about the desired defining_lineage
or color
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re: colour, here's how the clades output looks with this change, which I think is appropriate given that recombinant is similar in spirit to "other". Prior to #67 clades without defined colours would have been greyscale, and recombinant was never assigned a colour, so this is consistent with that outcome. However I have bigger questions about how we treat the "recombinant" category...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code only expects a boolean defining_lineage
for "other" at the moment when processing pango_lineages
, so an additional one-line patch is needed. (However also see the wider-ranging discussion elsewhere in the PR.)
@@ -23,7 +23,7 @@ def order_lineages(lineages, aliasor):
def lineage_to_clade(lineage, aliasor, fallback, clade_definitions):
lineage_full = aliasor.uncompress(lineage)
for clade_data in clade_definitions:
- if clade_data['clade']=='other':
+ if clade_data['clade']=='other' or clade_data['clade']=='recombinant':
continue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current PR works well in the context of nextstrain_clades
however I think the existance of "recombinant" causes a mismatch between this and pango_lineages
which I want to point out here. For the latter, we try to assign lineages to their respective nextstrain clade with "other" being the fallback. This means no lineages will be assigned to "recombinant". I can see a few different options here:
- Modify the upstream data categorization so that what's currently "recombinant" is grouped together with "other". This seems the most sensible to me but I am not an expert on the ingest side of things here.
- Modify the lineage-colouring script so that lineages which are not part of a named nextstrain clade are assigned to "other" or "recombinant" by checking if the pango lineage starts with "X".
- Do nothing -- perhaps this doesn't matter, especially if we use black as the colour assigned to "recombinant" and grey for "other" then the lineages look like this:
(I assume it's XBC* and XCH which are grouped as "recombinant" in the clades output)
Ah, this was the direction @trvrb was going in #45. I can close this PR in favor of grouping "recombinant" into "other". |
Adding "recombinant" to clade definition to fix the AssertionError raised by our automated model runs.
Follow up to #67
Checklist