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

Changed to decoupler for pseudobulking (#141) #153

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

alitinet
Copy link
Contributor

@alitinet alitinet commented Feb 3, 2023

No description provided.

@alitinet alitinet requested a review from Zethson February 3, 2023 14:08
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@github-actions github-actions bot added the enhancement New feature or request label Feb 3, 2023
@review-notebook-app
Copy link

review-notebook-app bot commented Feb 3, 2023

View / edit / reply to this conversation on ReviewNB

Zethson commented on 2023-02-03T14:13:21Z
----------------------------------------------------------------

Uhm, why are they all outcommented?


alitinet commented on 2023-02-03T14:28:26Z
----------------------------------------------------------------

oh thanks, forgot to remove, we don't need it any more

@review-notebook-app
Copy link

review-notebook-app bot commented Feb 3, 2023

View / edit / reply to this conversation on ReviewNB

Zethson commented on 2023-02-03T14:13:22Z
----------------------------------------------------------------

Line #2.    adata_pb = dc.get_pseudobulk(adata, sample_col='sample', groups_col='cell_type', layer='counts', min_prop=0.2, min_smpls=3)

Quite a long line, I'd add a line break after every parameter.


@review-notebook-app
Copy link

review-notebook-app bot commented Feb 3, 2023

View / edit / reply to this conversation on ReviewNB

Zethson commented on 2023-02-03T14:13:23Z
----------------------------------------------------------------

Line #1.    sc.pp.normalize_total(adata_pb, target_sum=1e4)

What made you do this? Think most normalize to millions?


alitinet commented on 2023-02-03T14:29:53Z
----------------------------------------------------------------

following decoupler tutorial here https://decoupler-py.readthedocs.io/en/latest/notebooks/pseudobulk.html

Zethson commented on 2023-02-03T14:31:05Z
----------------------------------------------------------------

I'd not change this without discussing it with Soroor

@review-notebook-app
Copy link

review-notebook-app bot commented Feb 3, 2023

View / edit / reply to this conversation on ReviewNB

Zethson commented on 2023-02-03T14:13:24Z
----------------------------------------------------------------

The dimensions are now very very different.

Before: 16 x 15710

Now: 16 x 2435

Intended? Could you explain this, please?


alitinet commented on 2023-02-03T14:41:34Z
----------------------------------------------------------------

It comes from https://decoupler-py.readthedocs.io/en/latest/generated/decoupler.get_pseudobulk.html#decoupler.get_pseudobulk params min_prop=0.2 and min_smpls=3, which filter out genes that are expressed in <20% of all cells and genes that are expressed in <3 samples

alitinet commented on 2023-02-03T14:43:41Z
----------------------------------------------------------------

I'm not sure if it'd better to make these more permissive

@review-notebook-app
Copy link

review-notebook-app bot commented Feb 3, 2023

View / edit / reply to this conversation on ReviewNB

Zethson commented on 2023-02-03T14:13:25Z
----------------------------------------------------------------

The new plot looks pretty different from the old one. What happened?


alitinet commented on 2023-02-03T14:41:49Z
----------------------------------------------------------------

Didn't notice, thanks! will fix

Copy link
Contributor Author

alitinet commented Feb 3, 2023

oh thanks, forgot to remove, we don't need it any more


View entire conversation on ReviewNB

Copy link
Contributor Author

alitinet commented Feb 3, 2023

following decoupler tutorial here https://decoupler-py.readthedocs.io/en/latest/notebooks/pseudobulk.html


View entire conversation on ReviewNB

Copy link
Member

Zethson commented Feb 3, 2023

I'd not change this without discussing it with Soroor


View entire conversation on ReviewNB

Copy link
Contributor Author

alitinet commented Feb 3, 2023

It comes from https://decoupler-py.readthedocs.io/en/latest/generated/decoupler.get_pseudobulk.html#decoupler.get_pseudobulk params min_prop=0.2 and min_smpls=3, which filter out genes that are expressed in <20% of all cells and genes that are expressed in <3 samples


View entire conversation on ReviewNB

Copy link
Contributor Author

alitinet commented Feb 3, 2023

Didn't notice, thanks! will fix


View entire conversation on ReviewNB

Copy link
Contributor Author

alitinet commented Feb 3, 2023

I'm not sure if it'd better to make these more permissive


View entire conversation on ReviewNB

@alitinet
Copy link
Contributor Author

alitinet commented Feb 3, 2023

Hey @soroorh, we changed to decoupler for pseudobulk creation which filters out genes that are expressed in <20% of all cells and genes that are expressed in < 3 samples. After this step, we are left with 2435 genes out of original 15710 genes. Do you think it would make sense to make this filtering step more permissive or is it ok as it is?

Also, for pb normalization, should we use 1e6 (as before) or 1e4 (from decoupler tutorial) as the normalizing factor?

@soroorh
Copy link
Contributor

soroorh commented Feb 3, 2023

Hey @alitinet. Since you are using filterByExpr from edgeR later in the workflow, I would retain as much genes as possible. So, I'd go with a more permissive filtering or no filtering. Also perhaps explicitly explain this in the chapter that if one is following edgeR's workflow for DE, they do not need to apply any filtering when making pseudo-bulks.

I would go with 1e6, as this is closer to how counts-per-million (CPM) is computed in edgeR.

@Zethson
Copy link
Member

Zethson commented Apr 12, 2023

@alitinet this chapter should then also use the pertpy dataloader for the kang dataset!

https://pertpy.readthedocs.io/en/latest/usage/data/pertpy.data.kang_2018.html#pertpy.data.kang_2018

@Zethson Zethson changed the base branch from development to master April 24, 2023 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants