Feature/temporal data - Githubissues

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:06Z ----------------------------------------------------------------

Please capitalize the title

Also would do real -> experimental

Marius1311 commented on 2023-06-13T11:26:26Z ----------------------------------------------------------------

Re capitalization: I would have to do that everywhere, will do that in a separate PR if required.

Marius1311 commented on 2023-06-13T11:26:41Z ----------------------------------------------------------------

Changed to "experimental"

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:07Z ----------------------------------------------------------------

This misses some introductory paragraph above.

Marius1311 commented on 2023-06-13T11:28:37Z ----------------------------------------------------------------

mh, ok, after the last review, I thought you wanted to have this message all the way to the top. Again, I would have to change this for all tutorials. Can be done on a separate PR.

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:08Z ----------------------------------------------------------------

Use ~ for the TemporalProblem

Remove "CellRank's"

Marius1311 commented on 2023-06-13T11:31:59Z ----------------------------------------------------------------

ok.

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:09Z ----------------------------------------------------------------

{mod}moscot

Cite moslin instead of the link to the repo?

Marius1311 commented on 2023-06-13T11:36:33Z ----------------------------------------------------------------

ok, fixed both.

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:10Z ----------------------------------------------------------------

Would replace ... -> :

No need for - in spatial-

"an scRNA-seq"

Can improve the accuracy instead?

Marius1311 commented on 2023-06-13T13:23:34Z ----------------------------------------------------------------

We know it improves the accuracy ;)

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:11Z ----------------------------------------------------------------

This points to master, should be main. For consistency, I'd include this under the 1st paragraph.

Marius1311 commented on 2023-06-13T13:24:32Z ----------------------------------------------------------------

ok.

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:11Z ----------------------------------------------------------------

Use subset_to_serum=True.

Marius1311 commented on 2023-06-13T13:30:30Z ----------------------------------------------------------------

That gives https://github.com/theislab/cellrank/issues/1053

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:12Z ----------------------------------------------------------------

Line #4.    # Experimental time points are saved in adata.obs["day"] as categoricals. Let's convert the categories to be numerical, this is required by moscot later on.

Please shorten the comment, it's overwhelmingly long.

_Marius1311 commented on 2023-06-13T13:31:32Z_ ----------------------------------------------------------------

ok.

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:14Z ----------------------------------------------------------------

Line #5.    adata.obs["day"] = adata.obs["day"].cat.rename_categories({day: float(day) for day in adata.obs["day"].cat.categories})

Use

adata.obs["day"].astype(float).astype("category")

Marius1311 commented on 2023-06-13T14:07:10Z ----------------------------------------------------------------

done.

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:15Z ----------------------------------------------------------------

Line #8.    adata.obs["day_numerical"] = adata.obs["day"].astype(float)

This shouldn't be needed.

_Marius1311 commented on 2023-06-13T13:32:28Z_ ----------------------------------------------------------------

Looks nicer - the color legend is continuous rather than discrete.

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:16Z ----------------------------------------------------------------

Line #10.    # Subsample to speed up the analysis - this tutorial is meant to run in a couple of minutes on a laptop. It’s not a problem for CellRank to do any of the computations here on the full data, we’d just have to wait a bit longer.

Please shorten the the comment or add as a normal text.

_Marius1311 commented on 2023-06-13T13:33:41Z_ ----------------------------------------------------------------

done.

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:16Z ----------------------------------------------------------------

Visualize the embedding

Marius1311 commented on 2023-06-13T13:34:02Z ----------------------------------------------------------------

ok.

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:17Z ----------------------------------------------------------------

I don't think link to FLE or the citation is needed.

Marius1311 commented on 2023-06-13T13:34:31Z ----------------------------------------------------------------

removed the link.

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:18Z ----------------------------------------------------------------

The link to neighbors doesn't work.

Marius1311 commented on 2023-06-13T13:35:13Z ----------------------------------------------------------------

I think I fixed it, should be "scanpy" rather than "sc".

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:19Z ----------------------------------------------------------------

"Waddington OT" -> "Waddington-OT" (as stylized in their publication)

Use {mod}moscot instead of the hardcoded link.

No - in uni-modal

Do not capitalize Transport

Marius1311 commented on 2023-06-13T13:36:37Z ----------------------------------------------------------------

ok.

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:20Z ----------------------------------------------------------------

No - in multi-modal

Please add nicer link to moscot, e.g.: "documentation <https://...>_ " .

Rather than linking to moslin, I think you should link to the moscot tutorial that uses the LineageProblem.

Please add a link in "... inculding many tutorials" to moscot's tutorials.

Marius1311 commented on 2023-06-13T13:40:45Z ----------------------------------------------------------------

all done.

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:21Z ----------------------------------------------------------------

Add 3 ~

you can use the ... or the LineageProblem, respectively.

Marius1311 commented on 2023-06-13T13:41:49Z ----------------------------------------------------------------

done.

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:22Z ----------------------------------------------------------------

Visualilze -> Visualize

Marius1311 commented on 2023-06-13T13:42:13Z ----------------------------------------------------------------

done.

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:23Z ----------------------------------------------------------------

local PCAs, that are computed separately

"comptes" -> "computes"

we'll use -> we use

Marius1311 commented on 2023-06-13T13:43:28Z ----------------------------------------------------------------

done.

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:24Z ----------------------------------------------------------------

"Early-" -> "early"

Marius1311 commented on 2023-06-13T13:43:50Z ----------------------------------------------------------------

ok.

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:24Z ----------------------------------------------------------------

"at the left marginal" -> "on the source marginal"

"regularitatition" -> "regularization"

Higher entropic regularization speeds up

Marius1311 commented on 2023-06-13T13:45:31Z ----------------------------------------------------------------

ok.

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:25Z ----------------------------------------------------------------

"to a Markov chain description" -> "to a Markov chain describing a biological sytem, we do the following:"

"arrive at a Markov-chain description" -> "and construct the Markov chain"

I'd add more information how the sparsification is done.

Marius1311 commented on 2023-06-13T13:48:05Z ----------------------------------------------------------------

ok, added a half sentence on sparsification.

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:26Z ----------------------------------------------------------------

Visualize the recovered dynamics

Marius1311 commented on 2023-06-13T13:48:34Z ----------------------------------------------------------------

ok.

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:27Z ----------------------------------------------------------------

Would do: by sampling {meth}random walks <cellrank.kernels.TransportMapKernel.plot_random_walks>.

Marius1311 commented on 2023-06-13T13:49:49Z ----------------------------------------------------------------

ok, good suggestion.

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:28Z ----------------------------------------------------------------

Please add a new paragraph (within the same cell) starting with: "Another way to visualize..."

Marius1311 commented on 2023-06-13T13:55:55Z ----------------------------------------------------------------

ok.

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:29Z ----------------------------------------------------------------

Can we please remove the vertical bars?

Marius1311 commented on 2023-06-13T13:56:33Z ----------------------------------------------------------------

I'm afraid I don't know how.

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:30Z ----------------------------------------------------------------

Line #12.    locs, labels = plt.xticks()

Can you try getting the ticks from ax?it will save us an import.

_Marius1311 commented on 2023-06-13T14:02:52Z_ ----------------------------------------------------------------

good point, found a solution.

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:31Z ----------------------------------------------------------------

Line #16.    plt.show()

Not need for this.

_Marius1311 commented on 2023-06-13T14:03:14Z_ ----------------------------------------------------------------

done.

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:32Z ----------------------------------------------------------------

This is not really needed here, would remove.

Marius1311 commented on 2023-06-13T14:03:32Z ----------------------------------------------------------------

ok.

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:15:33Z ----------------------------------------------------------------

Too many "to"s in "to learn how to use the transition matrix to"

"we recommend..." -> "we recommend to:" + "go through"/"take a look"/"read the Waddington-OT"

"parameter values" -> "parameters"

Would add the citations for WOT/moscot when you mention them, not at the end + use {mod}moscot.

Marius1311 commented on 2023-06-13T14:06:25Z ----------------------------------------------------------------

ok, done almost all.

michalk8 commented 1 year ago

@Marius1311 generally, would prefer these 2 things:

name the files as 100_initial_terminal.ipynb or 200_experimental_time.ipynb so that globs will sort it by their name
when adding additional resources (such as images), also prefix them with 100_, so that we know to which tutorials they belong.

review-notebook-app[bot] commented 1 year ago

View / edit / reply to this conversation on ReviewNB

michalk8 commented on 2023-06-13T00:23:17Z ----------------------------------------------------------------

All the headings, except the 1st one (CellRank meets real time) are too high (should be lower than the 1st one).

Marius1311 commented on 2023-06-13T11:31:04Z ----------------------------------------------------------------

ok, changed that.

Marius1311 commented 1 year ago

Re capitalization: I would have to do that everywhere, will do that in a separate PR if required.

View entire conversation on ReviewNB

Marius1311 commented 1 year ago

Changed to "experimental"

View entire conversation on ReviewNB

Marius1311 commented 1 year ago

mh, ok, after the last review, I thought you wanted to have this message all the way to the top. Again, I would have to change this for all tutorials. Can be done on a separate PR.

View entire conversation on ReviewNB

Marius1311 commented 1 year ago

ok, changed that.

View entire conversation on ReviewNB

Marius1311 commented 1 year ago

ok.

View entire conversation on ReviewNB

Marius1311 commented 1 year ago

ok, fixed both.

View entire conversation on ReviewNB

Marius1311 commented 1 year ago

We know it improves the accuracy ;)