Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assorted fixes for the AggregatedDotPlot class. #121

Open
wants to merge 10 commits into
base: devel
Choose a base branch
from
Open

Conversation

LTLA
Copy link
Member

@LTLA LTLA commented Nov 25, 2024

There are some implications for iSEE core that I don't have time to fix myself but someone might want to have a look at:

  • We're missing a fallback .multiSelectionResponsive() method for all Panels. Some default method would be helpful, possibly just returning TRUE and leaving it to subclasses to optimize. (Cleaned up .multiSelectionResponsive code and documentation. iSEE#684)
  • The contents of dims in ?.multiSelectionResponsive docs are not mentioned. This is a character vector containing either, both or none of "row" and "column", depending on the types of transmitted selections. (Cleaned up .multiSelectionResponsive code and documentation. iSEE#684)
  • In one of my commits, I set a cap on the number of rows used in the plot. This prevents the app from freezing when asked to cluster too many rows. However, this feature should rightfully be pushed into .extractAssaySubmatrix(), so that (i) the same general idea can be used for ComplexHeatmapPlot, and (ii) we get better memory efficiency by doing head() on .chosen.rows before coercing the matrix to a dense form.

LTLA added 8 commits November 24, 2024 22:31
This prevents the application from choking when it is asked to cluster too many
rows, e.g., because the user was careless in the table filtering used to
generate the multiple selection. Besides, there's no point making a dot plot
where you can't even identify the genes.
@LTLA LTLA requested a review from kevinrue November 25, 2024 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant