-
Notifications
You must be signed in to change notification settings - Fork 20
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
Update RKI data processing #476
Conversation
This is currently taking the first lineage value from the JSON blob. We may need to revisit this to loop and extract the latest lineage value.
@@ -287,6 +288,8 @@ def __init__(self): | |||
def transform_value(self, entry: dict) -> dict: | |||
entry['sequence'] = entry['sequence'].replace('\n', '') | |||
entry['length'] = len(entry['sequence']) | |||
lineage_dict = json.loads(entry['pango_lineage']) | |||
entry['pango_lineage'] = lineage_dict[0]['lineage'] |
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.
Out of interest, what does entry['pango_lineage']
look like?
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.
Sure thing! From here, the original format looks like
[{'method': 'PANGOLIN_LATEST', 'classification_version': 'PUSHER-v1.28.1', 'tool_version': '4.3', 'lineage': 'BA.2', '@qc_notes': 'Ambiguous_content:0.02', '@is_designated': False, '@qc_status': 'pass', '@conflict': 0.0, '@note': 'Usher placements: BA.2(1/1)'}]
Or a few examples from the data:
pango_lineages |
---|
[{"classification_version": "PUSHER-v1.29", "lineage": "BA.2", "tool_version": "4.3", "method": "PANGOLIN_LATEST", "@is_designated": false, "@qc_status": "pass", "@conflict": 0.0, "@qc_notes": "Ambiguous_content:0.02", "@note": "Usher placements: BA.2(1/1)"}] |
[{"classification_version": "PUSHER-v1.29", "lineage": "BA.2.9", "tool_version": "4.3", "method": "PANGOLIN_LATEST", "@is_designated": false, "@qc_status": "pass", "@conflict": 0.0, "@qc_notes": "Ambiguous_content:0.02", "@note": "Usher placements: BA.2.9(1/1)"}] |
[{"classification_version": "PUSHER-v1.29", "lineage": "BA.2", "tool_version": "4.3", "method": "PANGOLIN_LATEST", "@is_designated": false, "@qc_status": "pass", "@conflict": 0.0, "@qc_notes": "Ambiguous_content:0.02", "@note": "Usher placements: BA.2(2/2)"}] |
which, after the change, get corrected to:
pango_lineages |
---|
"BA.2" |
"BA.2.9" |
"BA.2" |
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.
Thanks! My only suggestion would be to do something like
assert len(lineage_dict)==1, f"RKI pango_lineage unexpectedly had more than one entry. Strain: {entry['name']}"
# (not sure if "name" is the correct key to use)
(also, perhaps lineage_dict
isn't the best name if it's a list)
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.
I might rename lineage_dict
to "lineage_json_blob" based on their own description (JSON-Blob
).
Instead of erroring out on multiple entries, I would move to looping through the entries to find the one with "method": "PANGOLIN_LATEST"
. We're not sure if they're setting this up to historical past pangolin lineage assignments, as lineage systems get updated.
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.
Heh, in the end, I went with your assert statement to at least flag when multiple entries start showing up (e57f3c0). This will help us double-check the purpose of the multiple entries, if they do eventually occur.
I double-checked entry.keys()
, and the entry['rki_accession']
is the strain ID.
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.
Hi, I just stumbled upon this issue, and we intend always to use PANGOLIN_LATEST for the latest assignment. So, you can filter the JSON array this way. I am sorry for not being more specific in our README. I will change that in our documentation. Please feel free to write us an email if you have questions igs-developers(at)rki.de .
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.
Thanks for chiming in @TheBready!
Tracking in #478
Based on <#476 (comment)>, we can filter for `PANGOLIN_LATEST` to get the latest lineage assignment. If none of the lineages are marked as `PANGOLIN_LATEST`, then just use the first one in the list since this was the behavior before the change. If there are multiple `PANGOLIN_LATEST` lineages, then just use the first one and output a warning. I've removed the assertion because this should not block the whole ncov-ingest workflow. Resolves <#478>
Description of proposed changes
Addressing: #475
Related issue(s)
Checklist