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

migrate to Seurat version 3 #94

Merged
merged 16 commits into from
Nov 26, 2019
Merged

migrate to Seurat version 3 #94

merged 16 commits into from
Nov 26, 2019

Conversation

TomKellyGenetics
Copy link

See #93 for previous discussions

@seb-mueller
Copy link
Collaborator

Thanks for migrating the PR!
Following the discussion on #93, this r.yaml content seem to work for me:
It's basically removing all version requirements:

channels:
  - conda-forge
  - bioconda
dependencies:
  - r
  - r-ggplot2
  - r-gridextra
  - r-viridis
  - r-stringdist
  - r-dplyr
  - r-mvtnorm
  - r-seurat=3
  - r-hmisc
  - r-tidyverse
  - r-devtools
  - r-rcolorbrewer
  - font-ttf-dejavu-sans-mono=2.37
  - fontconfig=2.13.1

@seb-mueller
Copy link
Collaborator

This seemed to have done the trick. The workflow went almost completly through, but raised an error in the `plot_violine.R':

Rscript --vanilla /home/travis/build/Hoohm/dropSeqPipe/.test/.snakemake/scripts/tmpeo8j9uf6.plot_violine.R
Activating conda environment: /home/travis/build/Hoohm/dropSeqPipe/.test/.snakemake/conda/f4398d85
In debug mode: saving R objects to inspect later
Error in `[.data.frame`(feature.names, , gene.column) : 
  undefined columns selected
Calls: Read10X -> [ -> [.data.frame

Not sure what happened there since I haven't tested your changes yet, do you have a way to test what's going on (it seems the read10x call is the culprit)?

@TomKellyGenetics
Copy link
Author

TomKellyGenetics commented Nov 24, 2019

I ran into the same issue. It seems that Read10X expects either features.tsv (and all files as .gz) or genes.tsv. Be default gene names are expected in column 2 but it is possible to change this.

Update: I think it may be easier to keep "genes" instead of "features" rather than compressing output files (as cellranger version 3 does). I have updated to accept "genes.tsv" with one column.

@TomKellyGenetics
Copy link
Author

Sorry to spam Travis with test jobs. I've narrowed down the issue, it seems that compress_mtx_summary only compresses the umi directory (not the reads).

Screen Shot 2019-11-25 at 15 44 47

rules/merge.smk Outdated Show resolved Hide resolved
@@ -219,7 +219,7 @@ ggsave(gg, file = snakemake@output$pdf_count_vs_gene, width = 12, height = 7)
# sample1_CAGCCCTCAGTA 264 437 sample1_CAGCCCTCAGTA sample1 CAGCCCTCAGTA 100 100 batch1 sample1 0.0389016 0.07551487 0.1144165 0.0228833 0.5102975 1.655303

# saving snakemake meta information into misc slot so all can be exported as one object
seuratobj@misc <- snakemake
Misc(object = pbmc_small, slot = "misc") <- snakemake
Copy link
Owner

Choose a reason for hiding this comment

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

What does this do?

I changed the snakemake object to a list since that's what the seurat object was asking but I haven't doublechecked how it looked like.

Copy link
Author

Choose a reason for hiding this comment

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

Seurat version 3 uses “Assay” slots so I think it needs to be specified differently. Their function seems to be recommended for this so they can update it if they change the object in future versions. The test environment on Travis threw an error on this and this fixes it.

@Hoohm
Copy link
Owner

Hoohm commented Nov 25, 2019

Not sure you saw it, but I sent you a PR on your branch. Could you merge it and PR it again? Sorry for the hassle, still struggling to keep PR authors properly in the history when merging.
TomKellyGenetics#1

@Hoohm
Copy link
Owner

Hoohm commented Nov 26, 2019

This looks nice.
@seb-mueller if you don't have any other requests, I'll merge it

@seb-mueller
Copy link
Collaborator

I've crudely tested it and it seems to work fine, so fine with me to merge it in.
@TomKellyGenetics, thanks so a lot for this, this is really helpful!

@Hoohm Hoohm merged commit c50c484 into Hoohm:develop Nov 26, 2019
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.

None yet

3 participants