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

feat: UI table formatting #950

Merged
merged 23 commits into from
Nov 5, 2024
Merged

Conversation

mattrunyon
Copy link
Collaborator

@mattrunyon mattrunyon commented Oct 22, 2024

Fixes #596. Adds UI table formatting based on the spec.

Features

  • Multiple formatting rules allowed on one table
  • Last matching rule for a formatting type is applied
  • Single dataclass (ui.TableFormat) which encompasses the potential formatting options
  • The cols parameter applies formatting to specific columns. The if_ parameter provides further filtering for rows
  • DH theme color keywords supported in addition to hex and CSS keyword colors
  • If a background color is specified with no text color, a contrasting black/white text color is applied

Missing

  • Docs
  • Unit tests
  • E2e tests
  • Testing/setting good error messages if if_ is bad. Other potential error points we might want to add
  • Number formatting rules can be applied to date columns since they're actually longs. I think this is fine as just something to note instead of trying to determine if a formatting option is date/number specific

Follow-ups

Example Code

from deephaven import ui
import deephaven.plot.express as dx

t = ui.table(
    dx.data.stocks(),
    format_=[
        ui.TableFormat(value="0.00%"),
        ui.TableFormat(cols="Timestamp", value="E, dd MMM yyyy HH:mm:ss z"),
        ui.TableFormat(cols="Size", color="info", if_="Size < 10"),
        ui.TableFormat(cols="Size", color="notice", if_="Size > 100"),
        ui.TableFormat(cols=["Sym", "Exchange"], alignment="center"),
        ui.TableFormat(
            cols=["Sym", "Exchange"], background_color="negative", if_="Sym=`CAT`"
        ),
        ui.TableFormat(background_color="positive", if_="Sym=`DOG`", color='negative')
    ],
)

@mattrunyon mattrunyon self-assigned this Oct 22, 2024
@mofojed
Copy link
Member

mofojed commented Oct 25, 2024

Default as a keyword for all values. Could be useful if you want to apply a background color and not a font color that may have no where clause later in the list. (follow-up PR?)

@mattrunyon Perhaps this could tie into the work for #549 (setting Undefined instead of None to get the default, for example). @wusteven815 will be working on that ticket soon.


> [!NOTE]
> The `where` property is a Deephaven query expression evaluated in the engine. You can think of it like adding a new boolean column using [`update_view`](https://deephaven.io/core/docs/reference/table-operations/select/update-view/)

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a sections for each formatter? Helps both SEO and shows a functional example of how to accomplish each task

#### Formatting table background colors

#### Formatting table decimals

#### Formatting table dates
...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it would make more sense to do Formatting Colors and Formatting Values unless you want each of those sections to be 1 or 2 sentences. That feels like overkill to me to show someone what "changing the background color to red" does or "changing text alignment to left".

I think formatting colors would explain what kind of color values you can use and what values can take colors.

Formatting values would link out to appropriate docs about value format strings (I think they're Java formatting strings, but need to verify)

Copy link
Contributor

Choose a reason for hiding this comment

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

Formatting colors and Formatting values sound reasonable in terms of avoiding super short sections; however, if a user is likely to search "Formatting table dates" and similar, it might be worth it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I ended up adding Formatting Colors as an h3 to describe you can (and should) use theme colors. Then text/background color as h4.

For values I added an h3 for numeric and datetime

Copy link
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

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

Looks good, a couple of minor fixes/notes. Would be nice if we could pass in a column name for the column or format value as well (with those taking precedent; e.g. if you have a column named RED, that value would take precedent over the colour RED).

I also tested with a large number of columns to make sure weren't getting caught with stuff out of the viewport, and it seems to work correctly:

from deephaven import empty_table, ui
colors=["RED", "GREEN", "BLUE"]
cols = [f"C_{i}=i" for i in range(1, 100)]
t = empty_table(1000).update([*cols, "x=i", "c=`` + colors[i%3]", "RED=`` + colors[(i + 1)%3]"])
ut = ui.table(t, formatting=[ui.TableFormat(where="C_3%2==0", color="RED")])

plugins/ui/docs/components/table.md Show resolved Hide resolved
plugins/ui/src/deephaven/ui/renderer/Renderer.py Outdated Show resolved Hide resolved
plugins/ui/src/deephaven/ui/components/table.py Outdated Show resolved Hide resolved
plugins/ui/src/js/src/elements/UITable/UITable.tsx Outdated Show resolved Hide resolved
plugins/ui/docs/components/table.md Outdated Show resolved Hide resolved
plugins/ui/docs/components/table.md Outdated Show resolved Hide resolved
plugins/ui/docs/components/table.md Outdated Show resolved Hide resolved

> [!NOTE]
> The `where` property is a Deephaven query expression evaluated in the engine. You can think of it like adding a new boolean column using [`update_view`](https://deephaven.io/core/docs/reference/table-operations/select/update-view/)

Copy link
Contributor

Choose a reason for hiding this comment

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

Formatting colors and Formatting values sound reasonable in terms of avoiding super short sections; however, if a user is likely to search "Formatting table dates" and similar, it might be worth it.

plugins/ui/docs/components/table.md Outdated Show resolved Hide resolved
plugins/ui/docs/components/table.md Outdated Show resolved Hide resolved
plugins/ui/docs/components/table.md Outdated Show resolved Hide resolved
plugins/ui/docs/components/table.md Outdated Show resolved Hide resolved
plugins/ui/docs/components/table.md Outdated Show resolved Hide resolved
mofojed
mofojed previously approved these changes Nov 1, 2024
Copy link
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

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

Looks good to me. Curious what @dsmmcken thinks of using if_ and format_

plugins/ui/src/deephaven/ui/components/table.py Outdated Show resolved Hide resolved
@dsmmcken
Copy link
Contributor

dsmmcken commented Nov 1, 2024

Curious what @dsmmcken thinks of using if_ and format_

I kinda hate it. I assume that's because those are both keywords in python? Did I miss a discussion somewhere?

dsmmcken
dsmmcken previously approved these changes Nov 1, 2024
Copy link
Contributor

@dsmmcken dsmmcken left a comment

Choose a reason for hiding this comment

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

Approving docs, not a fan of if_ or format_ but I defer to you guys. Pending accepting @margaretkennedy changes.

I wouldn't hate cond=

@mattrunyon mattrunyon dismissed stale reviews from dsmmcken and mofojed via 778ee38 November 4, 2024 16:50
margaretkennedy
margaretkennedy previously approved these changes Nov 4, 2024
@mattrunyon mattrunyon merged commit b9109e0 into deephaven:main Nov 5, 2024
17 checks passed
@mattrunyon mattrunyon deleted the ui-table-formatting branch November 5, 2024 20:53
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.

ui.table Formatting
4 participants