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

Update consensus cell type module to output one file for each library #118

Merged
merged 14 commits into from
Feb 5, 2025

Conversation

allyhawkins
Copy link
Member

Closes #114

Here I'm porting over the changes made in AlexsLemonade/OpenScPCA-analysis#1010 to output consensus cell types for each library in a separate TSV file. I copied over the script that is in OpenScPCA-analysis repo and made the appropriate changes to the workflow to have just the single process for assigning labels. I also added the blueprint reference as a parameter and updated the link for the consensus reference since that was slightly modified.

I ran this using the testing profile locally (thanks for setting that up!) and it completed successfully. I'll do a full testing run with simulated data once I file this PR and will request review once that run completes.

@jashapiro
Copy link
Member

This looks good from a first glance, but I wonder if we want to wait for the point release of openscpca-analysis to put this in so we can add in the versioned docker image? It might make it easier to reference the reference files as well (more human-readable, anyway).

@allyhawkins
Copy link
Member Author

This looks good from a first glance, but I wonder if we want to wait for the point release of openscpca-analysis to put this in so we can add in the versioned docker image? It might make it easier to reference the reference files as well (more human-readable, anyway).

Yes I was just starting to work on getting things done for the release, so that is my plan.

@allyhawkins
Copy link
Member Author

Noting that the test run worked successfully, but I want to wait to get this in after we do a release of OpenScPCA-analysis which is dependent on fixing the failed workflow run. So putting this on hold temporarily.

@allyhawkins
Copy link
Member Author

@jashapiro I updated this to use the versioned image. I'm running a test run now, but this should be ready for a formal review.

@allyhawkins allyhawkins requested a review from jashapiro February 4, 2025 15:31
Copy link
Member

@jashapiro jashapiro 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 good to me. The main change that I suggested is using the git tag for the reference files, as I think it will be easier to see if it is up to date as needed.

Other than that (and a minor formatting suggestions), I think this should be good to go.

@@ -19,5 +19,5 @@ params{
seurat_conversion_container = 'public.ecr.aws/openscpca/seurat-conversion:v0.2.0'
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to update all modules? Seems like we might want to, but we could do that in a separate PR

@allyhawkins
Copy link
Member Author

Noting that I made the changes you suggested and test run completed successfully, so going to go ahead and merge this.

@allyhawkins allyhawkins merged commit 004a165 into main Feb 5, 2025
3 checks passed
@allyhawkins allyhawkins deleted the allyhawkins/output-consensus-by-library branch February 5, 2025 15:51
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.

Modify consensus module to output results at a sample level
2 participants