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

Allow displayed diversity data to be downloaded #1144

Merged
merged 1 commit into from
May 29, 2020

Conversation

jameshadfield
Copy link
Member

We frequently get requests to share the data displayed in the
diversity panel. Here we address this by allowing download of
the data behind the graph. This is the simplest implementation,
as to download all data together ({events,entropy}U{nt,aa})
would require recalculations within the download logic as these
data are not stored by auspice when not being displayed.

closes #1143

Copy link
Contributor

@eharkins eharkins left a comment

Choose a reason for hiding this comment

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

Looks good, just a few tiny comments. Tested downloads on the review app which seem to be working.

I didn't go as far as validating the downloaded data was accurate, although I was confused why downloading in AA mode for dengue/all seemed to be missing a few codons. This seems like if anything it's probably related to that data and not to this PR:

gene	position	entropy
2K	5	0.007
2K	8	0.925
2K	11	0.380
2K	12	0.303
2K	13	0.925

export const entropyTSV = (dispatch, filePrefix, entropy, mutType) => {
const lines = mutType === "nuc" ? ["base"] : ["gene\tposition"];
lines[0] += entropy.showCounts ? "\tevents" : "\tentropy";
entropy.bars.forEach((bar) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

@jameshadfield if the entropy view is "zoomed" aka only showing a subset of sites, do we want to limit the download to just those sites? Defaulting to all of them seems ok to me since getting more data that you want is the worst case, but just wanted to check.

Copy link
Member

Choose a reason for hiding this comment

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

My guess is that downloading the data shown on the screen (filtered by currently visible tips) is the better / more powerful behavior. Just need to be clear to the user that this is what's going on. It matches the way that NEWICK download works as well.

Copy link
Member

Choose a reason for hiding this comment

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

I think the newick download is always the entire tree. but we have an option to download only the metadata for the displayed tips.

There are two different zooms that are relevant ehre. I think @eharkins is talking about the fraction of the genome that the zoom currently shows, while @trvrb talks about diversity among currently zoomed/selected tips in the tree (correct me if I am wrong). In the former case, more data doesn't hurt. In the latter case the zoom/filter defines what diversity is. so here providing the current state is most sensible but it needs to be obvious IMHO...

Copy link
Contributor

Choose a reason for hiding this comment

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

eharkins is talking about the fraction of the genome that the zoom currently shows

Yes.

trvrb talks about diversity among currently zoomed/selected tips in the tree (correct me if I am wrong). In the former case, more data doesn't hurt.

👍

In the latter case the zoom/filter defines what diversity is. so here providing the current state is most sensible but it needs to be obvious IMHO...

The current state of this is similar to how it is done for downloading other data that is filtered according to visible tree trips:
Screen Shot 2020-05-28 at 3 22 47 PM

Copy link
Member Author

Choose a reason for hiding this comment

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

the fraction of the genome that the zoom currently shows

I didn't take this into account. We could, if desired.

diversity among currently zoomed/selected tips in the tree

This is taken into account, as it defines the underlying diversity data. I dynamically change the message printed in the download modal to indicate this. For instance, changing to nucleotide, events, and zoomed to a sub-tree will display "The data behind the diversity panel showing a count of changes across the tree per nucleotide. Restricted to strains which are currently displayed (n = 134/543)."

Copy link
Member

Choose a reason for hiding this comment

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

Yes. I was confused. I was thinking of the "selected metadata". I would just export the entire genome, rather than the in view portion.

src/components/download/downloadModal.js Outdated Show resolved Hide resolved
We frequently get requests to share the data displayed in the
diversity panel. Here we address this by allowing download of
the data behind the graph. This is the simplest implementation,
as to download all data together ({events,entropy}U{nt,aa})
would require recalculations within the download logic as these
data are not stored by auspice when not being displayed.
@jameshadfield jameshadfield temporarily deployed to auspice-download-entrop-mduduq May 29, 2020 01:41 Inactive
@jameshadfield
Copy link
Member Author

I was confused why downloading in AA mode for dengue/all seemed to be missing a few codons

No -- this is a good point to raise. We only report values for variable positions, which is why there are "missing" codons. I'll make sure to add this to the docs when I write them.

@jameshadfield jameshadfield merged commit eefb707 into master May 29, 2020
@jameshadfield jameshadfield deleted the download-entropy-data branch May 29, 2020 01:42
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.

Allow export (download) of entropy & events
4 participants