-
Notifications
You must be signed in to change notification settings - Fork 25
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
MethylVI batch effect tutorial #363
base: main
Are you sure you want to change the base?
MethylVI batch effect tutorial #363
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
@@ -0,0 +1,468 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ori-kron-wis can we use our download function instead?
@@ -0,0 +1,468 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jointly = one model but treat both seperately not add both and get one output? Maybe write "we'll jointly model both".
How would modalities={"categorical_covariate_keys": "mCG"}
look like for two different fields. Please put in the comment.
Reply via ReviewNB
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make the description clearer, I changed the wording to "Here we'll jointly model both CpG and non-CpG methylation features" as suggested.
For the modalities
argument, I added a {note} block similar to the one in the totalVI tutorial (https://docs.scvi-tools.org/en/stable/tutorials/notebooks/multimodal/totalVI.html)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add the code that you would use if mCH and mCH in the note block below. It doesn't become clear to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe you post here the code if both have different categories and then I can advise what's the minimum information required.
@@ -0,0 +1,468 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe show get_normalized_methylation or DE additionally to go beyond latent space analysis? See our other tutorials for examples.
Reply via ReviewNB
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once this notebook is merged in I plan to create a separate tutorial for feature-level analysis that will replicate the results from our (upcoming) preprint
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have it in one notebook? I don't like bloating the number of tutorials. It's currently already quite painful to rerun and manually merge notebooks. Do you need different datasets for it?
See comments overall looks fine. @ori-kron-wis can you help make the style consistent with other tutorials (I assume removing notebook style commands). These might change rendering on the webpage, which can end up ugly. |
4bb1cbf
to
fa8837d
Compare
4f6af86
to
d077c0a
Compare
Tutorial accompanies scverse/scvi-tools#2834
Formatting
#
) headerReproducibility
scvi.settings.seed = 0
at the beginning of the notebookOther