-
Notifications
You must be signed in to change notification settings - Fork 17
chore: DH-21376: Add feature plan for UI table format heatmaps #1301
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
base: main
Are you sure you want to change the base?
Conversation
|
ui docs preview (Available for 14 days) |
mofojed
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Get some input from @dsmmcken as well.
How should we handle background vs. foreground coloring? Options include:
I threw in another options - passing ui.TableHeatmap in as the color or background_color property. I think that actually fits the best from an API point of view, but am welcome to holes being punched in my view.
Also, it leaves mode as an odd way to use TableDatabars.... I think we need to settle this before we merge the TableDatabars PR just to make sure we get it right.
I don't like separate booleans, and the apply_to is a little bit janky.
Heatmaps vs databars on the same column
Right now the priority would be determined by having separate TableFormat with a data bar specified in one, heatmap specified in another, and the last one taking priority. I think ordering of the TableFormat should be the only thing, and shouldn't need to worry about ambiguity within the TableFormat.
|
|
||
| Defer for now. Named color scales are just predefined lists of hex colors that would be passed to the `colors` parameter. The api shape doesn't need to change to support them, we'd just expose constants. | ||
|
|
||
| How should named color scales be specified? using strings (e.g., 'viridis') or provide a module of built-in scales organized by type, such as dh.ui.colors.sequential and dh.ui.colors.diverging |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can specify the colour scale with a string, they could map to those constants. Remember that the colour scales themselves should use our Deephaven colour variables (so they update correctly if the user changes the theme) I think.
| ui.TableHeatmap(mid=0, colors=["blue-600", "white", "red-600"]) | ||
|
|
||
| # Temperature centered on avg_temp | ||
| ui.TableHeatmap(mid=avg_temp, colors=["blue-400", "white", "red-400"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can auto min/max things for databars, should we auto mid things for table heatmap? Find the average of the column and adjust automatically
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the default mid should just be the midpoint of the range. If the range is -100 to +100, but it's heavily skewed towards the top end, the average could be like 80 and the heatmap might not make as much sense?
| Defaults to "background". | ||
| """ | ||
|
|
||
| type: str = field(default="heatmap", init=False) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't really played with field in DataClasses. Neat.
| max: ColumnName | float | None = None | ||
| mid: float | None = None | ||
| colors: list[Color] | list[tuple[float, Color]] | None = None | ||
| apply_to: Literal["background", "text"] | None = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't particularly like this apply_to thing... in terms of API, it would almost make sense to allow TableHeatmap as a value on color or background_color instead of just accepting Color... then you don't have a conflict between color and setting apply_to="color"... hmm. As it is now, I would default to background as well.
| 1. Add `TableHeatmap` dataclass to `table.py` | ||
| [ ] Add fields: `type`, `min`, `max`, `mid`, `colors`, `apply_to` | ||
| [ ] `type` is auto-populated `"heatmap"`, not user-settable | ||
| [ ] Widen `TableFormat.mode` to accept `TableHeatmap` | ||
|
|
||
| 2. Add validation in `table.__init__` | ||
| [ ] `colors` must have at least 2 entries when provided | ||
| [ ] `cols` required when `mode` is set (extend existing databar validation) | ||
|
|
||
| 3. Add `HeatmapConfig` TypeScript type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd probably specify to write the unit tests here as well, and make sure they pass/iterate until they do.
It doesn't say when in the plan it will add the unit tests.
|
I find the idea from @mofojed interesting of heatmap being a color style. If you look at how we handled heatmap in legacy tables, it was a background color. So the question is, do we think heatmap is a rendering mode or just a colorway? Databar is a rendering mode because it has special rendering outside of just the text (the bar, markers, offsetting text). Another mode option we've briefly discussed is If we go color (which right now is what I'm leaning towards), we should use auto contrasting text color if an explicit color or range isn't provided. Could there be a reason to have combined heatmap + databars in a column? I'm sure somebody could come up with a reason like using the databar as some sort of percentage indicator while the background color indicates how positive/negative that individual value was. |
This pr introduces a new heatmap api that aligns with
ui.TableFormatand uses themodeparameter, similar to how databars are set up.A few areas I’d especially like feedback on:
In addition to evenly spaced color lists, the api allows explicit (position, color) tuples (e.g. placing white at 20% instead of the midpoint). This slightly expands scope but feels useful for real-world cases and most visualization libraries seem to support some form of explicit color stops
How should we handle background vs. foreground coloring? Options include:
apply_toproperty ("background" or "text")Heatmaps vs databars on the same column:
@mofojed @mattrunyon