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

Prevent exploratory heatmaps that are too large from rendering #671

Merged
merged 4 commits into from
Jul 26, 2022

Conversation

rabstejnek
Copy link
Collaborator

@rabstejnek rabstejnek commented Jul 25, 2022

The computed matrixDataset is the first hurdle when it comes to rendering large heatmaps: the fix here is to split the HeatmapDatastore constructor so that a few properties set in the constructor are used to determine if the heatmap is too large, while the initialize method continues the computation intensive initialization.

image

Note:

  • It still hangs a little to perform the basic calculations to determine table size when the table is REALLY large, but it should not freeze the page.

@rabstejnek rabstejnek requested a review from shapiromatron July 25, 2022 20:14
Copy link
Owner

@shapiromatron shapiromatron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me, but I made a few changes in review that I was hoping you could take a look at.

  • I tried to approach this more as a guarding method. Previously, we rendered a no dataset warning before, and I just added this below that
  • I moved all the logic into the store
  • I also moved the hasDataset and settingsHash logic into the store instead of the view too

One bonus change, I renamed "X fields" and "Y fields" to "Columns" and "Rows", because that seems much more intuitive given that this is really just a big 2d table.

@rabstejnek if you're ok with my revisions, feel free to merge, else we can discuss more!

@rabstejnek rabstejnek merged commit 5302377 into main Jul 26, 2022
@rabstejnek rabstejnek deleted the explore-heatmap-fix branch July 26, 2022 14:14
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.

2 participants