Skip to content

Conversation

@mattrunyon
Copy link
Collaborator

Fixes #984

Allows all string format values to instead specify a column as the source. Ensures the column is always fetched and throws if the column is not a string type (can cause some really bad performance issues if value formatting is invalid). Also modified to just resolve all theme colors since this change could lead to unresolved theme colors in a column source.

The example in the docs or this example shows the feature working. Change the String cast on line 6 to non-string to test the error is thrown and displayed.

This example uses an input table joined to the table so you can adjust the formatting colors via input table (something Raffi asked about specifically)

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

_stocks = dx.data.stocks()
_source = _stocks.select_distinct("Sym").update("Color=(String)null")

color_source = input_table(init_table=_source, key_cols="Sym")

t = ui.table(
    _stocks.natural_join(color_source, "Sym", "SymColor=Color"),
    hidden_columns=["SymColor"],
    format_=[
        ui.TableFormat(cols="Sym", background_color="SymColor")
    ],
)

@mattrunyon mattrunyon requested a review from a team November 11, 2024 21:02
@mattrunyon mattrunyon self-assigned this Nov 11, 2024
@mattrunyon mattrunyon requested review from bmingles and removed request for a team November 11, 2024 21:02
const stableArray = useMemo(() => array, [...array]);
return stableArray;
const stableArray = useRef<T[]>(array);
if (!array.every((v, i) => v === stableArray.current[i])) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious what this improves over the previous version?

Copy link
Collaborator Author

@mattrunyon mattrunyon Nov 12, 2024

Choose a reason for hiding this comment

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

The previous version was actually a React error if the length of the array changed. useMemo dependency array is supposed to be a constant length, so this should allow for variable length arrays.

I will probably be replacing this with a useDeepEquals in a future PR that also handles objects

Copy link
Contributor

@bmingles bmingles Nov 12, 2024

Choose a reason for hiding this comment

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

Ah makes sense. Forgot about the length nuance. With that in mind, you could optimize this a bit more by checking if the length has changed before traversing the array.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (!array.every((v, i) => v === stableArray.current[i])) {
if (array.length !== stableArray.length || !array.every((v, i) => v === stableArray.current[i])) {

Copy link
Contributor

@bmingles bmingles left a comment

Choose a reason for hiding this comment

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

1 suggestion on error message. Otherwise, code looks good. Verified the String + non-string cases.

Copy link
Contributor

@bmingles bmingles left a comment

Choose a reason for hiding this comment

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

1 more suggestion to optimize the array memoization

Copy link
Contributor

@bmingles bmingles left a comment

Choose a reason for hiding this comment

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

LGTM

@mattrunyon mattrunyon merged commit c25f578 into deephaven:main Nov 12, 2024
@mattrunyon mattrunyon deleted the ui-table-formatting-column-source branch November 12, 2024 23: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 format values from a column

3 participants