Skip to content

improve performant table charts, remove feat flag #6892

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Oct 29, 2025
Merged

improve performant table charts, remove feat flag #6892

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7817a4d
fix time charts having too small bars, and better opacity hover
Light2Dark Oct 23, 2025
6d18fb7
soft remove feat flag use & args, enable string charts for <20k rows …
Light2Dark Oct 23, 2025
cc3425c
add sample notebook
Light2Dark Oct 23, 2025
dc5118e
increase chart width, fix stats
Light2Dark Oct 24, 2025
4d82ad5
update snapshot
Light2Dark Oct 24, 2025
2b75e9c
use right constant for test
Light2Dark Oct 24, 2025
41eacfb
remove pixi chg
Light2Dark Oct 27, 2025
7d52121
enable fallback for bin_values for table charts (#6957)
Light2Dark Oct 29, 2025
77b477c
rebase and update comment
Light2Dark Oct 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
657 changes: 473 additions & 184 deletions frontend/src/components/data-table/__tests__/__snapshots__/chart-spec-model.test.ts.snap
View file
Open in desktop

Large diffs are not rendered by default.

60 changes: 32 additions & 28 deletions frontend/src/components/data-table/__tests__/chart-spec-model.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ describe("ColumnChartSpecModel", () => {

it("should create an instance", () => {
const model = new ColumnChartSpecModel(
mockData,
[],
mockFieldTypes,
mockStats,
mockBinValues,
Expand All @@ -78,7 +78,7 @@ describe("ColumnChartSpecModel", () => {

it("should return header summary with spec when includeCharts is true", () => {
const model = new ColumnChartSpecModel(
mockData,
[],
mockFieldTypes,
mockStats,
mockBinValues,
Expand All @@ -93,7 +93,7 @@ describe("ColumnChartSpecModel", () => {

it("should return header summary without spec when includeCharts is false", () => {
const model = new ColumnChartSpecModel(
mockData,
[],
mockFieldTypes,
mockStats,
mockBinValues,
Expand All @@ -106,17 +106,17 @@ describe("ColumnChartSpecModel", () => {
expect(numberSummary.spec).toBeUndefined();
});

it("should return null spec for string and unknown types", () => {
it("should return null spec for unknown types", () => {
const model = new ColumnChartSpecModel(
mockData,
[],
mockFieldTypes,
mockStats,
mockBinValues,
mockValueCounts,
{ includeCharts: true },
);
const stringSummary = model.getHeaderSummary("string");
expect(stringSummary.spec).toBeNull();
const unknownSummary = model.getHeaderSummary("unknown");
expect(unknownSummary.spec).toBeNull();
});

it("should handle special characters in column names", () => {
Expand All @@ -126,11 +126,18 @@ describe("ColumnChartSpecModel", () => {
const specialStats: Record<ColumnName, Partial<ColumnHeaderStats>> = {
"column.with[special:chars]": { min: "2023-01-01", max: "2023-12-31" },
};
const specialBinValues: Record<ColumnName, BinValues> = {
...mockBinValues,
"column.with[special:chars]": [
{ bin_start: "2023-01-01", bin_end: "2023-06-01", count: 10 },
{ bin_start: "2023-06-01", bin_end: "2023-12-31", count: 20 },
],
};
const model = new ColumnChartSpecModel(
mockData,
[],
specialFieldTypes,
specialStats,
mockBinValues,
specialBinValues,
mockValueCounts,
{ includeCharts: true },
);
Expand All @@ -139,17 +146,17 @@ describe("ColumnChartSpecModel", () => {
expect(
// @ts-expect-error layer should be available
(summary.spec?.layer[0].encoding?.x as { field: string })?.field,
).toBe("column\\.with\\[special\\:chars\\]");
).toBe("bin_start");
});

it("should expect bin values to be used for number and integer columns when feat flag is true", () => {
it("should expect bin values to be used for number and integer columns", () => {
const model = new ColumnChartSpecModel(
mockData,
[],
mockFieldTypes,
mockStats,
mockBinValues,
mockValueCounts,
{ includeCharts: true, usePreComputedValues: true },
{ includeCharts: true },
);
const summary = model.getHeaderSummary("number");
expect(summary.spec).toBeDefined();
Expand All @@ -171,24 +178,21 @@ describe("ColumnChartSpecModel", () => {
expect(summary2.spec?.data?.values).toEqual(mockBinValues.integer);
});

it("should handle datetime bin values when feat flag is true", () => {
it("should handle datetime bin values", () => {
const model = new ColumnChartSpecModel(
mockData,
[],
mockFieldTypes,
mockStats,
mockBinValues,
mockValueCounts,
{ includeCharts: true, usePreComputedValues: true },
{ includeCharts: true },
);

const summary = model.getHeaderSummary("datetime");
expect(summary.spec).toBeDefined();
// @ts-expect-error data.values should be available
expect(summary.spec?.data?.values).toEqual(mockBinValues.datetime);

// Expect hconcat since there are nulls
// @ts-expect-error hconcat should be available
expect(summary.spec?.hconcat).toBeDefined();
expect(summary.spec).toMatchSnapshot();

// Test again without the nulls
Expand All @@ -197,12 +201,12 @@ describe("ColumnChartSpecModel", () => {
datetime: { min: "2023-01-01", max: "2023-12-31" },
};
const model2 = new ColumnChartSpecModel(
mockData,
[],
mockFieldTypes,
mockStats2,
mockBinValues,
mockValueCounts,
{ includeCharts: true, usePreComputedValues: true },
{ includeCharts: true },
);
const summary2 = model2.getHeaderSummary("datetime");
expect(summary2.spec).toBeDefined();
Expand All @@ -212,14 +216,14 @@ describe("ColumnChartSpecModel", () => {
expect(summary2.spec?.hconcat).toBeUndefined();
});

it("should handle boolean stats when feat flag is true", () => {
it("should handle boolean stats", () => {
const model = new ColumnChartSpecModel(
mockData,
[],
mockFieldTypes,
mockStats,
mockBinValues,
mockValueCounts,
{ includeCharts: true, usePreComputedValues: true },
{ includeCharts: true },
);
const summary = model.getHeaderSummary("boolean");
expect(summary.spec).toBeDefined();
Expand All @@ -234,14 +238,14 @@ describe("ColumnChartSpecModel", () => {
expect(summary.spec).toMatchSnapshot();
});

it("should handle string value counts when feat flag is true", () => {
it("should handle string value counts", () => {
const model = new ColumnChartSpecModel(
mockData,
[],
mockFieldTypes,
mockStats,
mockBinValues,
mockValueCounts,
{ includeCharts: true, usePreComputedValues: true },
{ includeCharts: true },
);
const summary = model.getHeaderSummary("string");
expect(summary.spec).toBeDefined();
Expand All @@ -262,7 +266,7 @@ describe("ColumnChartSpecModel", () => {
expect(summary.spec).toMatchSnapshot();
});

describe("snapshot", () => {
describe("snapshot with legacy data spec", () => {
const fieldTypes: FieldTypes = {
...mockFieldTypes,
a: "number",
Expand Down
Loading
Loading