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

Adding ReadAlevin function [WIP] #896

Closed
wants to merge 5 commits into from
Closed

Conversation

k3yavi
Copy link
Member

@k3yavi k3yavi commented Oct 25, 2018

Hi Seurat team,
First of all thanks for the awesome tool, it has helped us and our user base a lot in analyzing the scRNA-seq data.

We have recently developed a dscRNA-seq quantification pipeline with name Alevin (preprint here), and we frequently encounter feature requests from our user base regarding the preprocessing of the reads for subsampling/batch correction like COMBINE-lab/salmon#305. We generally recommend users to go through this tutorial to parse the Alevin output and perform bias correction through Seurat package downstream of Alevin. In lieu of that to simplify the clustering pipeline by one more step we'd love to make the following pull request to parse the Alevin output and would really appreciate, if possible, to optimize the committed parsing function and merge to master in your next release.

Alevin dumps both csv and a more efficient binary format. We are working on writing a parser for the binary format and would update this pull request soon. As a noob in R I'd appreciate if you see something horribly wrong in the committed parsing code and would appreciate your feedback.

evolvedmicrobe and others added 5 commits July 16, 2018 16:51
If the data used for the regression contains NA values, this will lead to errors downstream.  For example, if one row contains an NA value, it will be removed and create a size mismatch when a data.frame is later constructed using the residuals of all rows, or if a linear model is used, it will create a problem when the QR values are reused.
Warn on NA data being used for regression
@mojaveazure mojaveazure changed the base branch from develop to release/3.0 October 25, 2018 21:35
@mojaveazure
Copy link
Member

Hi Avi,

I know this is a work in progress, but could you make sure you're building off of the release/3.0 branch? We're revamping the Seurat object system and how Seurat is organized, so we'd like all incoming pull requests to target this new scheme.

@k3yavi
Copy link
Member Author

k3yavi commented Oct 25, 2018

Sounds good @mojaveazure , thanks for the heads up !

@k3yavi
Copy link
Member Author

k3yavi commented Oct 30, 2018

Hi @mojaveazure , I have the relevant changes ready in the forked seurat repo in an updated release/3.0 branch. Should I make a new pull request or push the changes into develop for which this pull request was made ?

@mojaveazure
Copy link
Member

Hi Avi, whichever's easiest for you. A new PR might be cleaner, but it's up to you.

@k3yavi
Copy link
Member Author

k3yavi commented Nov 1, 2018

Thanks @mojaveazure , I'll make a new one then and close this one.

@k3yavi k3yavi closed this Nov 1, 2018
@k3yavi k3yavi deleted the develop branch November 1, 2018 13:06
dcollins15 added a commit that referenced this pull request Feb 15, 2024
Fixes for SCTransform using `vars.to.regress`
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

5 participants