search

theislab / cellrank_notebooks

Tutorials and examples for CellRank.
https://cellrank.org
BSD 3-Clause "New" or "Revised" License
6 stars 5 forks source link

Finish the CytoTRACE tutorial #51

Closed Marius1311 closed 2 years ago

Marius1311 commented 2 years ago

closes #50

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

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

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

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2022-06-14T14:18:39Z ----------------------------------------------------------------

Extra space in implementation .

Also, don't like stuff like using ... (prefer :) + similar comments for PT tutorial appply


Marius1311 commented on 2022-06-14T16:34:48Z ----------------------------------------------------------------

These extra spaces always come from citations which haven't been rendered yet.

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

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2022-06-14T14:18:40Z ----------------------------------------------------------------

Could you maybe make a), b) as list?


Marius1311 commented on 2022-06-14T16:35:25Z ----------------------------------------------------------------

yes.

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

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2022-06-14T14:18:41Z ----------------------------------------------------------------

Would call it pseudotime tutorial instead of PseudotimeKernel tutorial


Marius1311 commented on 2022-06-14T16:35:44Z ----------------------------------------------------------------

ok.

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

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2022-06-14T14:18:41Z ----------------------------------------------------------------

In the PT tutorial, you use &,here and, please ensure consistency.


Marius1311 commented on 2022-06-14T16:36:18Z ----------------------------------------------------------------

wow you're being rigurous! But okay.

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

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2022-06-14T14:18:42Z ----------------------------------------------------------------

Do we really need numpy import? Think you can just run .sum on the object below when computing the HVGs.


Marius1311 commented on 2022-06-14T16:37:44Z ----------------------------------------------------------------

you're right, I got rid of this import.

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

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2022-06-14T14:18:43Z ----------------------------------------------------------------

Would maybe remove this and make the path explicit in the function call.


Marius1311 commented on 2022-06-14T16:38:18Z ----------------------------------------------------------------

ok, I removed the text.

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

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2022-06-14T14:18:44Z ----------------------------------------------------------------

Please use cr.kernels.CytoTRACEKernel,not the import (again to maintain consistency).


Marius1311 commented on 2022-06-14T16:39:01Z ----------------------------------------------------------------

I think it's good if users see different ways of doing this! Both ways work, I'll leave this as is.

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

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2022-06-14T14:18:45Z ----------------------------------------------------------------

I would move this operation to the above cell as

ctk = CytoTRACEKernel(adata).compute_cytotrace()

Otherwise, I feel like this sentence and the above are a bit short.


Marius1311 commented on 2022-06-14T16:40:17Z ----------------------------------------------------------------

ok.

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

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2022-06-14T14:18:46Z ----------------------------------------------------------------

kNN + would cite La Manno for the RNA velocity


Marius1311 commented on 2022-06-14T16:41:20Z ----------------------------------------------------------------

ok.

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

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2022-06-14T14:18:47Z ----------------------------------------------------------------

Would consider linking to every kernel you mention here.


Marius1311 commented on 2022-06-14T16:48:30Z ----------------------------------------------------------------

ok!

Marius1311 commented 2 years ago

These extra spaces always come from citations which haven't been rendered yet.

View entire conversation on ReviewNB

Marius1311 commented 2 years ago

yes.

View entire conversation on ReviewNB

Marius1311 commented 2 years ago

ok.

View entire conversation on ReviewNB

Marius1311 commented 2 years ago

wow you're being rigurous! But okay.

View entire conversation on ReviewNB

Marius1311 commented 2 years ago

you're right, I got rid of this import.

View entire conversation on ReviewNB

Marius1311 commented 2 years ago

ok, I removed the text.

View entire conversation on ReviewNB

Marius1311 commented 2 years ago

I think it's good if users see different ways of doing this! Both ways work, I'll leave this as is.

View entire conversation on ReviewNB

Marius1311 commented 2 years ago

ok.

View entire conversation on ReviewNB

Marius1311 commented 2 years ago

ok.

View entire conversation on ReviewNB

Marius1311 commented 2 years ago

ok!

View entire conversation on ReviewNB

Marius1311 commented 2 years ago

Thanks for your review @michalk8, should be done now.

  • © Githubissues.
  • Githubissues is a development platform for aggregating issues.