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

Add support for "data_provenance" metadata #1313

Merged
merged 2 commits into from
Mar 30, 2021
Merged

Conversation

jameshadfield
Copy link
Member

We now support data provenance to be defined in the dataset JSON
(see nextstrain/augur#705) and all core
nCoV builds have been updated to include this here. This commit
parses and renders such information.

Note that a previous commit removed the "Build info" from the byline
for datasets displaying GISAID (this commit), which I believe was an
oversight. This commit reinstates it.

Examples:

 "data_provenance": [
      {
        "name": "GISAID"
      },
      {
        "name": "COG UK",
        "url": "https://www.cogconsortium.uk"
      },
      {},
      {
        "name": "source with invalid link",
        "url": "/bad.link"
      },
      {
        "name": "source without link"
      }
    ],

image

Rendering of /ncov/global with this PR:
image

For comparison, rendering of /ncov/global as of v2.24.1 (parent):

image

In the early stages of COVID-19, we added support for acknowledging
GISAID as the source of data in the Byline. This was inferred based
on domain / dataset name heuristics.

We now support data provenance to be defined in the dataset JSON
(see nextstrain/augur#705) and all core
nCoV builds have been updated to include this here. This commit
parses and renders such information.

Note that a previous commit removed the "Build info" from the byline
for datasets displaying GISAID (see [0]), which I believe was an
oversight. This commit reinstates it.

[0] 18d5d21
@jameshadfield jameshadfield requested a review from huddlej March 29, 2021 01:03
@jameshadfield jameshadfield temporarily deployed to auspice-data-provenance-fi082r March 29, 2021 01:03 Inactive
@trvrb
Copy link
Member

trvrb commented Mar 29, 2021

This looks great to me @jameshadfield. Thanks so much for implementing. The link parsing is a nice touch. This is good to go from my perspective.

Copy link
Contributor

@huddlej huddlej left a comment

Choose a reason for hiding this comment

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

This worked for me, exploring the ncov builds in the Heroku deployment. We don't currently link out to GISAID from the button, but there was a comment in the original issue about linking from the given image to GISAID's site. Is that functionality we still want? Should it be in a separate PR, so this most important functionaity can get this out as soon as possible?

@emmahodcroft
Copy link
Member

Good catch @huddlej . My expectation is that GISAID would likely prefer if that logo links to their website. But that might take a bit of fiddling, I agree that getting this live first and then working to incorporating the linking would be best, so that other builds can implement this quickly.

@trvrb
Copy link
Member

trvrb commented Mar 29, 2021

Nice catch! I didn't realize that the link out to gisaid.org was missing from the live version as well. We should make this addition. It won't be hard.

This adds a link to gisaid.org from the GISAID logo (when present). It also adjusts parsing so that data_provenance.name == "gisaid" will still get picked up.
@trvrb trvrb temporarily deployed to auspice-data-provenance-fi082r March 29, 2021 18:43 Inactive
@trvrb
Copy link
Member

trvrb commented Mar 29, 2021

Okay. This commit adds a link to gisaid.org and will also catch data_provenance.name = gisaid (which seemed like it could be a common input). I think this can be merged now.

@jameshadfield
Copy link
Member Author

Thanks all ⭐

@jameshadfield jameshadfield merged commit 305b1ab into master Mar 30, 2021
@jameshadfield jameshadfield deleted the data-provenance branch March 30, 2021 00:59
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.

4 participants