search

theislab / scib-reproducibility

Additional code and analysis from the single-cell integration benchmarking project
https://theislab.github.io/scib-reproducibility/
MIT License
52 stars 14 forks source link

Adding files from scib #4

Closed mumichae closed 3 years ago

mumichae commented 3 years ago

Scripts added:

Corresponding PRs:

mumichae commented 3 years ago

@martaint Is it safe to move all the R/visualization code here or are you still working on it?

martaint commented 3 years ago

I think it's safe! I'm not working on it :)

On Sat, 6 Mar 2021, 06:05 Michaela Mueller notifications@github.com wrote:

@martaint https://github.com/martaint Is it safe to move all the R/visualization code here or are you still working on it?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/theislab/scib-reproducibility/pull/4#issuecomment-791874602, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHR32OQTFPRXYMLCF3BZLB3TCGZX7ANCNFSM4YVK5JYQ .

mumichae commented 3 years ago

Wow, that's a lot of notebooks! I assume somebody has already checked these are all still relevant?

We removed notebooks some time back (PR), so I think things should be OK. @LuckyMD @danielStrobl Could you double-check if there are any notebooks we don't need anymore?

* Do we need the `.ipynb_checkpoints` directories?

Good point, didn't realise that I was adding those. They should be removed and ignored now.

Should we move the data files in the visualization/ directory to the top data/ directory? At least those that are for the paper not just examples for the plotting functions (@martaint might be able to help with that).

I agree, that would be better. If we move the plot data, we just need to update the R scripts accordingly.

LuckyMD commented 3 years ago

Things I think might be removable:

  1. The empty file @lazappi highlighted
  2. Mouse atlas notebooks (@danielStrobl and @mumichae)
  3. notebooks/Visualization_ATAC.ipynb from @kridsadakorn
  4. notebooks/silhouette_graph.ipynb, notebooks/trajectory_plots and notebooks/visualization.ipynb from @danielStrobl
  5. visualization/atlas_heatmap.ipynb from @mumichae

Would love to hear if you think these can be removed or if this is important for reproducibility. If important, then maybe we need a further folder.

danielStrobl commented 3 years ago

The mouse atlas notebooks can be removed I think. I would keep the trajectory_plots and visualisation notebooks as the figures for the supplementary were generated with those. The silhouette graph notebook can be removed.

danielStrobl commented 3 years ago

One thing I noticed is that the notebooks/visualisation.ipynb in master is not the most recent one, it's actually in the recompute_cluster_metrics branch

LuckyMD commented 3 years ago

Thanks for comments @danielStrobl. Could you say how you would name the folder (or in which existing folder) the notebooks to keep should be put? Just visualization?

danielStrobl commented 3 years ago

Yes, sounds good

lazappi commented 3 years ago

One thing I noticed is that the notebooks/visualisation.ipynb in master is not the most recent one, it's actually in the recompute_cluster_metrics branch

@danielStrobl Can you open a separate PR for the latest version (or add it here)? Or open an issue I guess, just don't want to forget about this.

P.S. I think you have a pending invite to join this repo, I can resend if you need.

danielStrobl commented 3 years ago

Ok, I'll open a PR for that. That would be great if you could resend the invite.

mumichae commented 3 years ago

I agree that the heatmap and mouse atlas notebooks can be removed.

mumichae commented 3 years ago

I removed the heatmap plotting script (duplicate) and moved the preprocessing scripts to the Masterinternship_2019 repository.

LuckyMD commented 3 years ago

also, new analysis notebook updates from scIB.

lazappi commented 3 years ago

Summarising things that have come up in comments:

Done

Todo

Anything that's incorrect or I missed?

Updated 2021-04-22

lazappi commented 3 years ago

Also just noticed that each of the dataset notebook folders has an empty README. We should probably either add some info about the datasets or just delete them. @LuckyMD what do you think?

LuckyMD commented 3 years ago

Could you copy over some text from the dataset description in the paper to the README?

lazappi commented 3 years ago

Could you copy over some text from the dataset description in the paper to the README?

That's definitely possible. Alternatively could describe what is in the notebooks (or both).

LuckyMD commented 3 years ago

Or maybe better: everyone could write a small blurb (3-4 sentences of what is in the dataset). I think that could even be from the SI sections.

review-notebook-app[bot] commented 3 years ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

lazappi commented 3 years ago

Did some of the minor things, see updated checklist https://github.com/theislab/scib-reproducibility/pull/4#issuecomment-820517623. Main things left to do are double checking notebooks are up to date (@danielStrobl can you help with that?) and deciding what to do with the READMEs.

martaint commented 3 years ago

@lazappi I added a file in scib (https://github.com/theislab/scib/pull/234) which should also be added here in the respective notebook folder. Could you take care of that? Don't want to mess with this PR :) thanks a lot!