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 generation of QC report to pipeline #31

Merged
merged 8 commits into from
Sep 17, 2021
Merged

Conversation

jashapiro
Copy link
Member

This PR adds a final step to the pipeline to run the QC generation function from scpcaTools. The process and pipeline are pretty simple, but there were a few changes I made along the way, some for clarity and some for bug fixes and some just because I was inspired and/or delusional

  • In testing, I ran into a few outstanding bugs in the pipeline (mostly related to alt experiments) that should now be fixed. (In addition to the bugs in scpcaTools itself that were fixed in that repo).
  • I altered the container we are using to track the edge release for now, so we will always be using the latest scpcaTools as we update things like the QC report. We will want to freeze this to a release when we start processing.
  • In a fit of late night hubris, I realized that since we now have to use an R image with R4.1 we could use native pipes instead of magrittr, so I did that, just to see if I could. Still works as before, but I could change it back... Or we could change all of scpcaTools to use it!

Closes #1

Copy link
Member

@allyhawkins allyhawkins left a comment

Choose a reason for hiding this comment

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

This looks pretty good to me. I think the only thing missing is the check for a missing sample ID in the sce_qc_report.R script. I feel like that is worthwhile to have. I'm fine with the other changes to the workflow and good catch on the bugs!

if(!file.exists(opt$filtered_sce)){
stop("Missing filtered .rds file")
}

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 we also need a check here to make sure that the sample Id isn't null? I don't think we want a default value for that so we probably want to add that in just in case.

Copy link
Member Author

Choose a reason for hiding this comment

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

For some reason, I thought that optparse would throw an error if no default was specified and no value given, but it turns out I am wrong about that. So I added the check you suggested.

Copy link
Member

@allyhawkins allyhawkins 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!

@jashapiro jashapiro merged commit b9ddb5b into main Sep 17, 2021
@jashapiro jashapiro deleted the jashapiro/1-generate_qc branch September 17, 2021 14:38
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.

Add generation of QC report to scpca nextflow pipeline
2 participants