Move accession to the first column of metadata_all.tsv #36
Conversation
During the merge of Usvi data and GenBank data, the accession field ended up as the last column. This caused confusion as the first column was named "genbank_accession" which could be mistaken for the strain ID. This commit moves the "accession" column to the first column such that "accession" and "genbank_accession" are next to each other; hopefully, providing clarity that "accession" is being used as the strain ID, while "genbank_accession" can be used to generate a url to the NCBI GenBank record if provided.
| | csvtk concat -tl - {input.usvi_metadata} \ | ||
| | tsv-select -H -f accession --rest last \ |
There was a problem hiding this comment.
(non-blocking)
Suggestion: In cases like this where a column name is ambiguous, add more detail somewhere in the repo. Maybe as the docstring of this rule:
rule append_usvi:
"""Appending USVI sequences.
Notable columns:
- accession: Either the GenBank accession or USVI accession.
- genbank_accession: For Auspice to generate a URL to the NCBI GenBank record. Empty for USVI sequences.
- url: ?
"""
input:
…
I don't know if this has been done in other repos, but it seems like it'd be useful to bring this context out of commit messages/PRs and into the code itself.
There was a problem hiding this comment.
Thanks, agree with writing the context into the code itself (docstring). Fixed with 3631e90 but let me know if the url explanation is confusing
There was a problem hiding this comment.
(Discussion is beyond the scope of this PR now, so feel free to merge first.)
The Auspice source code indicates that when both are specified, genbank_accession takes precedence and url will be ignored.
Because of this behavior, I don't think url should be set for GenBank sequences. That would only cause confusion in the event that GenBank changes the URL, we try to update it in this file, and scratch our heads over why the old URL is still showing on Auspice. I would expect url to only be set for USVI sequences where there is no genbank_accession.
In cases like this where a column name is ambiguous ('accession' and 'genbank_accession'),
bring this context out of commit messages/PRs and into the code itself.
Description of proposed changes
During the merge of Usvi data and GenBank data, the
accessionfield ended up as the last column. This caused confusion as the first column was namedgenbank_accessionwhich could be mistaken for the strain ID.This commit moves the
accessioncolumn to the first column such thataccessionandgenbank_accessionare next to each other; hopefully, providing clarity thataccessionis being used as the strain ID, whilegenbank_accessioncan be used to generate a URL (in auspice) to the NCBI GenBank record if provided.Related issue(s)
Checklist