chanzuckerberg / single-cell-data-portal

The data portal supporting the submission, exploration, and management of projects and datasets to cellxgene.
MIT License
61 stars 12 forks source link

Disable cell ontology indentation and sort with hierarchical clustering by default #3847

Closed atarashansky closed 1 year ago

atarashansky commented 1 year ago

Because we will soon aggregate gene expressions and cell counts across all descendants for each node in the ontology, the current approximated view of the ontology structure via an indented flat list can be misleading. As a result, we decided to disable this and sort with hierarchical clustering by default. We're also removing the ability to change the structure dropdown entirely.

Please ping @pablo-gar @atarashansky @hthomas-czi @signechambers1 and @ambrosejcarr to review when this is available.

seve commented 1 year ago

Hierarchical clustering cell ordering currently depends on genes being inputted, which leaves the y-axis unpopulated until a gene is entered. How do we want to handle this?

@pablo-gar @atarashansky @hthomas-czi @signechambers1 @ambrosejcarr

pablo-gar commented 1 year ago

I would strongly recommend we address the original issue after we have the aggregate gene expressions in place.

Hierarchical clustering cell ordering currently depends on genes being inputted, which leaves the y-axis unpopulated until a gene is entered. How do we want to handle this?

For this I'd say we preserve the order without indentation. Or we have a default order using 1 marker gene per cell type and perform clustering on that (I think this is great option if users don't want to use hierarchical clustering at all) , that will lead to a very nice data-based ordering

seve commented 1 year ago

@pablo-gar you're recommending we address aggregation before we do the work detailed in this ticket?

atarashansky commented 1 year ago

@pablo-gar I think using the current cell type ordering without indentations by default would be a good compromise.

seve commented 1 year ago

@atarashansky: @pablo-gar I think using the current cell type ordering without indentations by default would be a good compromise.

So we temporarily show current ordering with no indentation, then switch to hierarchical after gene(s) are added?

pablo-gar commented 1 year ago

Two things being discussed:

  1. When to implement what's detailed in this ticket
  2. After we implement what's detailed in this ticket, how do we handle the case when a tissue is selected but no genes are selected

For 1. CL-based aggregate gene expressions was decided to be a more accurate implementation than non-aggregates, however having indentations would make it confusing (for reasons I won't go into here), thus this ticket. Given that logic, imo this ticket should be implemented concurrently to the aggregation, not before the aggregation.

For 2. I'd recommend the default is the current ordering with no indentations, once a gene is selected then it goes to hierarchical clustering.

seve commented 1 year ago

Cool, that's what I've implemented. I'll wait for aggregation to merge.

https://user-images.githubusercontent.com/8716829/211687789-76246ac8-bd64-42ea-822a-2bc890a85203.mov

signechambers1 commented 1 year ago

@seve @atarashansky I want to make sure we have a lot of eyes on this for review before it hits prod. I dont think the one day review in staging is enough. What are our options for reviewing? I want to review the aggregation + front end work together if possible with you both and @pablo-gar and @hthomas-czi, then Ambrose.

signechambers1 commented 1 year ago

To clarify the closing of this ticket, right now this feature is behind a feature flag, not live for users. @atarashansky what is the feature flag info?

atarashansky commented 1 year ago

Sorry, this ticket was in "Ready for prod" so I must have closed it with all the other tickets there in bulk.

The feature flag info is is_rollup=true