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

Include attribution link with gisaid_epi_isl #1382

Merged
merged 1 commit into from
Jul 26, 2021
Merged

Conversation

trvrb
Copy link
Member

@trvrb trvrb commented Jul 24, 2021

Description of proposed changes

This updates behavior to match what we were doing for genbank_accession. Here, we special case both genbank_accession and gisaid_epi_isl and construct direct DB links when these are present rather than using the more generic accession / url metadata pair.

This update allows users to lookup attribution info for individual samples even if workflow metadata did not include this attribution data.

Testing

Tested locally.

@jameshadfield jameshadfield temporarily deployed to auspice-attribution-lin-m4hice July 24, 2021 17:29 Inactive
This updates behavior to match what we were doing for genbank_accession. We special case both genbank_accession and gisaid_epi_isl and construct direct DB links when these are present rather than using the accession / url metadata pair.

This update allows users to lookup attribution info for individual samples even if workflow metadata did not include this attribution data.
@trvrb trvrb temporarily deployed to auspice-attribution-lin-m4hice July 24, 2021 17:56 Inactive
Copy link
Member

@jameshadfield jameshadfield left a comment

Choose a reason for hiding this comment

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

Tested using a variety of (core) datasets to check the different formats data may appear in (partly to remind myself!)

Fields example behavior
(none) H7N9 Nothing displayed
url locally tested "Strain URL" displayed with link to supplied url
accession H3N2 Displayed without any links
accession, url Historical nCoV dataset display the accession which is a link to the specified URL
genbank_accession, url ncov/open "Genbank Accession" displayed, with link constructed from the accession number. The provided url information is ignored.
gisaid_epi_isl, url ncov/gisaid "GISAID EPI ISL" displayed, with a link to the epicov JSON. The provided url information is ignored.
genbank_accession, gisaid_epi_isl, url ncov/gisaid "Genbank accession" and "GISAID EPI ISL" displayed, each with a link as above. The provided url information is ignored.

@trvrb if this is as you expect it to be, please merge & I can release in this week's auspice release.

@trvrb
Copy link
Member Author

trvrb commented Jul 26, 2021

Thanks for outlining James. We may want to rethink how we do accessions at some point to make it easier to have multiple accessions with matched URLs for a sample, but I think this current priority system is good for the moment. genbank_accession and gisaid_epi_isl take precedence over more generic accession / url inclusion.

@trvrb trvrb merged commit 8c28394 into master Jul 26, 2021
@trvrb trvrb deleted the attribution-link branch July 26, 2021 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants