Closed zhangjiaobxy closed 3 years ago
Hi @zhangjiaobxy ,
thanks for a really nice bug-report, I am able to reproduce this in 1.2.0
! If you set cr.settings.verbosity = 4
before cr.pl.terminal_states(adata, discrete=True)
, you should see something like:
DEBUG: Unable to set attribute `._absorption_probabilities`, skipping
DEBUG: Unable to set attribute `._diff_potential`, skipping
DEBUG: Expected lineage names to be of length `3`, found `4`. Creating new names # this is the problem
DEBUG: Expected lineage colors to be of length `3`, found `3`. Creating new colors # there was a typo, should say: found `4`
We've fixed a part of this problem recently in 1.3.1
, specifically in https://github.com/theislab/cellrank/pull/556 , it determined the number of terminal states based on adata.obsm['macrostates_fwd']
, which is still 3
. This however fixes only the 1st error (overwriting) and it will warn wrong #colors/names is found.
The 2nd error seems to still be present (calling cr.pl.terminal_states
in succession) because of the inconsistency (4 categories in adata.obs['terminal_states']
, but still only 3 columns in adata.obsm['macrostates_fwd']
and only the latter is being checked.
So it happily creates just 3 names/colors (cr.pl.terminal_states(adata, discrete=False)
works and shows only these 3 macrostates), but when plotting these as discrete, the created color mapper only maps 3 names to colors: https://github.com/theislab/cellrank/blob/master/cellrank/tl/estimators/_property.py#L441 (it silently fails because Epsilon
is cut out).
I've fixed this in #563 (still need to write tests though).
@Marius1311 we should refactor the estimator (by removing _property.py
+ defining a state machine) so that it's easier to handle (fix bugs, extend, ...). I will create an issue.
Thanks @michalk8 for your quick reply. I am glad to hear that you found the causes so fast. Please feel free to let me know when you fix it. I am happy to test the code. :)
Hi @michalk8, thanks for taking the initiative here - feel free to draft a refactoring of the estimators and we can discuss it!
Hi @zhangjiaobxy, I believe the bug you reported is fixed in #563, if you don't mind, would be great if you could test this!
Thanks @Marius1311. I can have a test. Should I test it under the developmental version?
pip install git+https://github.com/theislab/cellrank@dev
@zhangjiaobxy
This is still not yet on dev
, pip install git+https://github.com/theislab/cellrank@fix_inconsistent_state
should do the trick.
It's not perfect, but should solve the issue you've had. Proper fix can/wlll be issues when dealing with #564 (without refactoring, it's currently not possible).
@zhangjiaobxy This is still not yet on
dev
,pip install git+https://github.com/theislab/cellrank@fix_inconsistent_state
should do the trick. It's not perfect, but should solve the issue you've had. Proper fix can/wlll be issues when dealing with #564 (without refactoring, it's currently not possible).
Thanks @michalk8 @Marius1311 ! I just tested this version. It works well. I guess the problem has been fixed. Thank you so much for your help!
closed via #563
Thanks a lot @zhangjiaobxy for testing the changes.
Dear CellRank Team,
Thanks for developing this amazing tool! I am following the
Kernels and estimators
tutorial, and I want to manually set the terminal states for my dataset.After I run
g.set_terminal_states
, I can seeadata.obs['terminal_states']
,adata.uns['terminal_states_colors']
, andadata.uns['terminal_states_names']
are all set to my manually defined terminal states. The problem is that when I plot the terminal statescr.pl.terminal_states(adata, discrete=True)
, theadata.obs['terminal_states']
,adata.uns['terminal_states_colors']
, andadata.uns['terminal_states_names']
are setting back to the originally computed terminal states.I tested the same code using the pancreas dataset
(datasets/endocrinogenesis_day15.5.h5ad)
, the error is reproducible. Here is the code:Then I check the automatically computed terminal states using the following code:
Here is the output:
Next, manually define/set terminal states:
Now, using the following code to check if the terminal states are manually set:
Here is the output:
Everything looks perfect so far, however, if I run this line of code![image](https://user-images.githubusercontent.com/14197636/114311624-7eb1a580-9abd-11eb-9662-81c6da83a6c4.png)
cr.pl.terminal_states(adata, discrete=True)
. I can get an expected figure.But if I check the terminal states again:
The terminal states are setting back to the original terminates states:
At this time, if I run this line of code
cr.pl.terminal_states(adata, discrete=True)
, I got the following error:Versions:
Please feel free to correct me if I do anything wrong. Thanks!