LTLA / scRNAseq

Clone of the Bioconductor repository for the scRNAseq package.
http://bioconductor.org/packages/devel/data/experiment/html/scRNAseq.html
24 stars 12 forks source link

Add spike in counts for Zeisel data #9

Closed alanocallaghan closed 4 years ago

alanocallaghan commented 4 years ago

As threatened promised on Bioc slack.

This uses a table each from Svensson's power analysis paper and the spike in mix concs from Thermo. https://www.nature.com/articles/nmeth.4220

Should I use BiocFileCache for the tables? I think I trust Nature to be stable, but I'd be inclined to include the file from Thermo in the repo, since manufacturers can be pretty bad for randomly restructuring their websites.

LTLA commented 4 years ago

Should I use BiocFileCache for the tables?

Yes. There shouldn't be added data files.

If you're concerned about the longevity of the spike-in concentration file, the better approach is to define an entirely new set of data.Rmd and metadata.R files to upload just that file to ExperimentHub, then modify .calculate_spikeins to be an exposed function that retrieves said file.

I don't see the need to have a separate inst/extdata/spike-concs.tsv file. Just hard-code the concentrations in the getter function. You would have to match studies manually anyway, I would not trust a grep to do the job.

alanocallaghan commented 4 years ago

Cool thanks, figured it wasn't the right approach but I'm not familiar with the bioc hub tools. I'll change this evening. once you're happy with the setup for zeisel I'll see about adding others

LTLA commented 4 years ago

Several points, in order:

Don't bother force pushing, I'm going to squash all the commits anyway.

alanocallaghan commented 4 years ago
  1. My bad, it's a mess because I didn't get to finishing edits (ran out of hours.on Sunday evening). Should only be one of course.
  2. Alright sounds more straightforward.
  3. I want to include that option to avoid having the same logic in every data file that includes spikes
  4. Works for me

Right you are, for some reason I thought the tsvs would end up in the history anyway but that's nonsense.

alanocallaghan commented 4 years ago

Just take the file as is and put it on ExperimentHub.

Does this entail making an ExperimentHub package, or do you mean using the existing link and adding metadata to EH? If the latter surely just BiocFileCache'ing is functionally the same.

LTLA commented 4 years ago

We will stick it onto EHub as its own entry so that it persists even after ThermoFisher changes its link.

alanocallaghan commented 4 years ago

Fair dues. Should I ping Lori about it then, or how does one upload?

LTLA commented 4 years ago

I'll handle it, I've got a whole bunch of EHub-related stuff to do anyway.

LTLA commented 4 years ago

But do clean up the remainders of what I suggested.

alanocallaghan commented 4 years ago

Yeah will do of course. Thanks for the feedback

On 4 Mar 2020 5:50 am, Aaron Lun notifications@github.com wrote:

But do clean up the remainders of what I suggested.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/LTLA/scRNAseq/pull/9?email_source=notifications&email_token=ACSHYKGGEGMWWZ644QSORCDRFXT2TA5CNFSM4K6W3IJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENWOTCI#issuecomment-594340233, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ACSHYKA774WAB2MCP3KDSRLRFXT2TANCNFSM4K6W3IJQ.

alanocallaghan commented 4 years ago

Fixed, though may need to fix the EHub download code if the metadata doesn't match.

alanocallaghan commented 4 years ago

Should be all good now, I've checked locally and both user-facing changes seem to work