Closed Marius1311 closed 1 year ago
Check out this pull request on
See visual diffs & provide feedback on Jupyter Notebooks.
Powered by ReviewNB
Hi @michalk8, for some reason, I cannot plot a circular projection here, it complains. KeyError: "Unable to find lineage data in
adata.obsm['lineages_fwd']."
But it's weird, because I computed fate probabilities above, it said Adding
adata.obsm['lineages_fwd']`, but when I check in AnnData, it's actually not there. Would be great if you could see whether this persists on your machine, the data is downloaded automatically from figshare.
Note: using the same CellRank commit, this worked in the fate probability tutorial.
Note: using the same CellRank commit, this worked in the fate probability tutorial.
Can you please check assert adata is pk.adata
(or assert g.adata is adata
)? I suspect some copy was made.
Otherwise will check as soon as I get back.
Update: the latter doesn't seem to hold, so a copy is being made somehow. Will fix.
Update v2: you initialize the GPCCA with pk
, not pk_new
; pk
is associated with the adata
before saving, so after fixing this, it works.
Amazing @michalk8, thank you so much for spotting this!
Hi @michalk8, first of all, thanks for your hint above, that fixed the problem with the circular projection. I re-formated the notebook using myst and in principle, it's ready for your review.
However, the pre-commit hooks were failing. Initially, isort complained with "RuntimeError: The Poetry configuration is invalid". I fixed that by pinning isort to version 5.12.0
in the .pre-commit-config.yaml
. However, now black-nb is failing with
Traceback (most recent call last):
File "/Users/marius/.cache/pre-commit/repo__m858xp/py_env-python3/bin/black-nb", line 8, in <module>
sys.exit(cli())
File "/Users/marius/.cache/pre-commit/repo__m858xp/py_env-python3/lib/python3.9/site-packages/click/core.py", line 1130, in __call__
return self.main(*args, **kwargs)
File "/Users/marius/.cache/pre-commit/repo__m858xp/py_env-python3/lib/python3.9/site-packages/click/core.py", line 1055, in main
rv = self.invoke(ctx)
File "/Users/marius/.cache/pre-commit/repo__m858xp/py_env-python3/lib/python3.9/site-packages/click/core.py", line 1404, in invoke
return ctx.invoke(self.callback, **ctx.params)
File "/Users/marius/.cache/pre-commit/repo__m858xp/py_env-python3/lib/python3.9/site-packages/click/core.py", line 760, in invoke
return __callback(*args, **kwargs)
File "/Users/marius/.cache/pre-commit/repo__m858xp/py_env-python3/lib/python3.9/site-packages/click/decorators.py", line 26, in new_func
return f(get_current_context(), *args, **kwargs)
File "/Users/marius/.cache/pre-commit/repo__m858xp/py_env-python3/lib/python3.9/site-packages/black_nb/cli.py", line 190, in cli
sources = black.get_sources(
File "src/black/__init__.py", line 637, in get_sources
TypeError: 'NoneType' object is not subscriptable
and I don't really get what it wants.
View / edit / reply to this conversation on ReviewNB
michalk8 commented on 2023-06-05T18:45:44Z ----------------------------------------------------------------
I think there's too much text, would shorten (before "In this tutorial") + please add more links (like cite when mentioning RNA velocity (La Manno, Bergen), developmental potentials (CytoTRACE publication), etc.
Please also link to kernels as, e.g., {mod}~cellrank.kernels
and {mod}cellrank.estimator
.
I'd move the "To demonstrate ..." to a cell above when you create the dataset + link to it using {mod}~cellrank.datasets.bone_marrow
.
I slightly prefer using "you will learn how to:" instead of "you will learn how to..." (intead of ..., would use :, also in other places).
Marius1311 commented on 2023-06-07T12:42:25Z ----------------------------------------------------------------
I agree with all and adapted all.
View / edit / reply to this conversation on ReviewNB
michalk8 commented on 2023-06-05T18:45:45Z ----------------------------------------------------------------
Use "mod}kernel
<cellrank.kernels>.
Marius1311 commented on 2023-06-07T12:43:57Z ----------------------------------------------------------------
done.
View / edit / reply to this conversation on ReviewNB
michalk8 commented on 2023-06-05T18:45:46Z ----------------------------------------------------------------
Dev no longer exists, please remove
Why do we need python-igraph
?
Marius1311 commented on 2023-06-07T15:30:01Z ----------------------------------------------------------------
I don't remember; I removed it for now.
View / edit / reply to this conversation on ReviewNB
michalk8 commented on 2023-06-05T18:45:47Z ----------------------------------------------------------------
Do we need all of these?
Marius1311 commented on 2023-06-07T15:39:47Z ----------------------------------------------------------------
We only require the UserWarning class, I removed the others.
View / edit / reply to this conversation on ReviewNB
michalk8 commented on 2023-06-05T18:45:48Z ----------------------------------------------------------------
{class}~anndata.AnnData
Marius1311 commented on 2023-06-07T15:41:51Z ----------------------------------------------------------------
ok
View / edit / reply to this conversation on ReviewNB
michalk8 commented on 2023-06-05T18:45:49Z ----------------------------------------------------------------
I'd prefer if this were on a lower section level (also in other places); currently the notebook feels flat.
Marius1311 commented on 2023-06-07T15:45:02Z ----------------------------------------------------------------
ok, I created some hierachy.
View / edit / reply to this conversation on ReviewNB
michalk8 commented on 2023-06-05T18:45:49Z ----------------------------------------------------------------
Missing `.
Marius1311 commented on 2023-06-07T15:45:38Z ----------------------------------------------------------------
ok, thanks.
View / edit / reply to this conversation on ReviewNB
michalk8 commented on 2023-06-05T18:45:50Z ----------------------------------------------------------------
Capitalize and link to Palantir
Extra space at the end before the dot.
Marius1311 commented on 2023-06-07T15:46:44Z ----------------------------------------------------------------
thanks.
View / edit / reply to this conversation on ReviewNB
michalk8 commented on 2023-06-05T18:45:52Z ----------------------------------------------------------------
When linking to the API, use just {mod}~cellrank.kernels
.
Marius1311 commented on 2023-06-07T15:47:22Z ----------------------------------------------------------------
great, thanks.
View / edit / reply to this conversation on ReviewNB
michalk8 commented on 2023-06-05T18:45:53Z ----------------------------------------------------------------
{class}~anndata.AnnData
Marius1311 commented on 2023-06-07T15:47:57Z ----------------------------------------------------------------
ok.
View / edit / reply to this conversation on ReviewNB
michalk8 commented on 2023-06-05T18:45:54Z ----------------------------------------------------------------
Add link to the transition matrix attribute as, e.g., {mod}~cellrank.kernels.PseudotimeKernel.transition_matrix
.
Marius1311 commented on 2023-06-07T15:49:10Z ----------------------------------------------------------------
ok.
View / edit / reply to this conversation on ReviewNB
michalk8 commented on 2023-06-05T18:45:56Z ----------------------------------------------------------------
Please use somewhere {meth}random walks <cellrank.kernels.PseudotimeKernel.plot_random_walks>
.
Marius1311 commented on 2023-06-07T15:50:32Z ----------------------------------------------------------------
done
View / edit / reply to this conversation on ReviewNB
michalk8 commented on 2023-06-05T18:45:57Z ----------------------------------------------------------------
Not sure whether I'd add these see alsos here or whether to group them all in the end - wdyt?
Marius1311 commented on 2023-06-07T15:50:55Z ----------------------------------------------------------------
I think they should remain within their corresponding sections.
View / edit / reply to this conversation on ReviewNB
michalk8 commented on 2023-06-05T18:45:58Z ----------------------------------------------------------------
Link to the PT kernel.
Marius1311 commented on 2023-06-07T15:51:46Z ----------------------------------------------------------------
ok.
View / edit / reply to this conversation on ReviewNB
michalk8 commented on 2023-06-05T18:45:59Z ----------------------------------------------------------------
simpley
Marius1311 commented on 2023-06-07T15:52:11Z ----------------------------------------------------------------
thanks for spotting.
View / edit / reply to this conversation on ReviewNB
michalk8 commented on 2023-06-05T18:46:00Z ----------------------------------------------------------------
{class}~anndata.AnnData
Marius1311 commented on 2023-06-07T15:52:47Z ----------------------------------------------------------------
ok.
View / edit / reply to this conversation on ReviewNB
michalk8 commented on 2023-06-05T18:46:01Z ----------------------------------------------------------------
I think this should not be here - it breaks the flow.
Marius1311 commented on 2023-06-07T15:53:26Z ----------------------------------------------------------------
Ok, moved it to the end.
View / edit / reply to this conversation on ReviewNB
michalk8 commented on 2023-06-05T18:46:02Z ----------------------------------------------------------------
Remove "fancy"
immediatly
Link to AnnData using {class}~anndata.AnnData
.Try also {mod}scanpy
.
Marius1311 commented on 2023-06-07T15:57:47Z ----------------------------------------------------------------
all done, thanks.
View / edit / reply to this conversation on ReviewNB
michalk8 commented on 2023-06-05T18:46:02Z ----------------------------------------------------------------
I'd link to the states as, e.g., {attr}initial <cellrank.estimators.GPCCA.initial_states>
, etc.
Marius1311 commented on 2023-06-07T16:03:45Z ----------------------------------------------------------------
all done.
View / edit / reply to this conversation on ReviewNB
michalk8 commented on 2023-06-05T18:46:03Z ----------------------------------------------------------------
Capitalize GPCCA, use ~
Missing dot at the end.
Marius1311 commented on 2023-06-07T16:04:43Z ----------------------------------------------------------------
done.
View / edit / reply to this conversation on ReviewNB
michalk8 commented on 2023-06-05T18:46:04Z ----------------------------------------------------------------
Visualise -> visualize
Link to circular projection.
Marius1311 commented on 2023-06-07T16:06:08Z ----------------------------------------------------------------
ok.
View / edit / reply to this conversation on ReviewNB
michalk8 commented on 2023-06-05T18:46:05Z ----------------------------------------------------------------
Colored by the cluster labels.
Marius1311 commented on 2023-06-07T16:06:38Z ----------------------------------------------------------------
mh, that I'm not sure about. I'll leave this.
View / edit / reply to this conversation on ReviewNB
michalk8 commented on 2023-06-05T18:46:06Z ----------------------------------------------------------------
Would remove "see the dedicated" part, won't read nicely once it renders. (also in other places).
Marius1311 commented on 2023-06-07T16:08:08Z ----------------------------------------------------------------
ok, removed everywhere.
View / edit / reply to this conversation on ReviewNB
michalk8 commented on 2023-06-05T18:46:07Z ----------------------------------------------------------------
I'd add this note to the top of every notebook, not the bottom.
Marius1311 commented on 2023-06-07T16:08:48Z ----------------------------------------------------------------
I disagree here - I don't think we should overload users with information at the beginning, but cut to the chase quickly, and reserve this info for people who made it all the way through.
Marius1311 commented on 2023-06-07T16:09:13Z ----------------------------------------------------------------
Ah, I see what you mean - you're just referring to the note. Ok, that's fine, happy to change that.
Marius1311 commented on 2023-06-07T16:09:53Z ----------------------------------------------------------------
done.
I think they should remain within their corresponding sections.
View entire conversation on ReviewNB
This serves as an entry point for CellRank analysis.