Skip to content

Commit

Permalink
[FIX] charts: fix trend line for datetime
Browse files Browse the repository at this point in the history
Task Description

This commit aims to fix a wrong computation for trend line when using
date (or datetime) as label. The issue can be reproduced following this:

1. Create a new spreadsheet
2. In A1:A4, enter 1/8, 1/9, 1/10 and 1/12
3. In B1:B4, enter 1, 4, 9 and 25
4. Create a new chart with A1:A4 as labels and B1:B4 as data series
5. Add a polynomial trend line of degree 2

We should have a perfect fit, as we have a quadradic progression with the
month number, but it's not the case

Related Task:

Task: 0
Part-of: #5069
Signed-off-by: Lucas Lefèvre (lul) <[email protected]>
  • Loading branch information
anhe-odoo committed Oct 25, 2024
1 parent 69a8602 commit d3a2181
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 39 deletions.
24 changes: 18 additions & 6 deletions src/helpers/figures/charts/chart_common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -534,31 +534,43 @@ export function interpolateData(
if (values.length < 2 || labels.length < 2 || newLabels.length === 0) {
return [];
}
const labelMin = Math.min(...labels);
const labelMax = Math.max(...labels);
const labelRange = labelMax - labelMin;
const normalizedLabels = labels.map((v) => (v - labelMin) / labelRange);
const normalizedNewLabels = newLabels.map((v) => (v - labelMin) / labelRange);
switch (config.type) {
case "polynomial": {
const order = config.order ?? 2;
if (order === 1) {
return predictLinearValues([values], [labels], [newLabels], true)[0];
return predictLinearValues([values], [normalizedLabels], [normalizedNewLabels], true)[0];
}
const coeffs = polynomialRegression(values, labels, order, true).flat();
return newLabels.map((v) => evaluatePolynomial(coeffs, v, order));
const coeffs = polynomialRegression(values, normalizedLabels, order, true).flat();
return normalizedNewLabels.map((v) => evaluatePolynomial(coeffs, v, order));
}
case "exponential": {
const positiveLogValues: number[] = [];
const filteredLabels: number[] = [];
for (let i = 0; i < values.length; i++) {
if (values[i] > 0) {
positiveLogValues.push(Math.log(values[i]));
filteredLabels.push(labels[i]);
filteredLabels.push(normalizedLabels[i]);
}
}
if (!filteredLabels.length) {
return [];
}
return expM(predictLinearValues([positiveLogValues], [filteredLabels], [newLabels], true))[0];
return expM(
predictLinearValues([positiveLogValues], [filteredLabels], [normalizedNewLabels], true)
)[0];
}
case "logarithmic": {
return predictLinearValues([values], logM([labels]), logM([newLabels]), true)[0];
return predictLinearValues(
[values],
logM([normalizedLabels]),
logM([normalizedNewLabels]),
true
)[0];
}
default:
return [];
Expand Down
6 changes: 3 additions & 3 deletions src/helpers/figures/charts/chart_common_line_scatter.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { ChartConfiguration, ChartDataset, LegendOptions } from "chart.js";
import { DeepPartial } from "chart.js/dist/types/utils";
import { BACKGROUND_CHART_COLOR, LINE_FILL_TRANSPARENCY } from "../../../constants";
import { toJsDate, toNumber } from "../../../functions/helpers";
import { toNumber } from "../../../functions/helpers";
import { Color, Format, Getters, Locale, Range } from "../../../types";
import {
AxisType,
Expand Down Expand Up @@ -166,8 +166,8 @@ export function getTrendDatasetForLineChart(
break;
case "time":
for (const point of dataset.data) {
const date = toJsDate({ value: point.x }, locale).getTime();
if (typeof point.y === "number") {
const date = toNumber({ value: point.x }, locale);
if (point.y !== null) {
filteredValues.push(point.y);
filteredLabels.push(date);
}
Expand Down
89 changes: 59 additions & 30 deletions tests/figures/chart/chart_plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3112,32 +3112,31 @@ describe("trending line", () => {
});
const runtime = model.getters.getChartRuntime("1") as LineChartRuntime;
const step = (6 - 1) / 25;
//@ts-ignore
const data = runtime.dataSetsValues[1].data;
for (let i = 0; i < data.lenght; i++) {
const value = data.lenght;
const data = runtime.chartJsConfig.data.datasets[1].data;
for (let i = 0; i < data.length; i++) {
const value = data[i];
const expectedValue = Math.pow(1 + i * step, 2);
expect(value).toEqual(expectedValue);
expect(value).toBeCloseTo(expectedValue);
}
});

test("trend line works with datetime values as labels", () => {
setFormat(model, "C1:C5", "m/d/yyyy");
mockChart();
const config = getChartConfiguration(model, "1");
expect(config.options.scales.x1).toMatchObject({
type: "category",
display: false,
offset: false,
labels: range(0, 26).map((v) => v.toString()),
});
const runtime = model.getters.getChartRuntime("1");
const step = (5 - 1) / 25;
//@ts-ignore
const data = runtime.dataSetsValues[1].data;
for (let i = 0; i < data.lenght; i++) {
const value = data.lenght;
const runtime = model.getters.getChartRuntime("1") as LineChartRuntime;
const step = (6 - 1) / 25;
const data = runtime.chartJsConfig.data.datasets[1].data;
for (let i = 0; i < data.length; i++) {
const value = data[i];
const expectedValue = Math.pow(1 + i * step, 2);
expect(value).toEqual(expectedValue);
expect(value).toBeCloseTo(expectedValue);
}
});

Expand All @@ -3149,24 +3148,24 @@ describe("trending line", () => {
offset: false,
labels: range(0, 26).map((v) => v.toString()),
});
const runtime = model.getters.getChartRuntime("1");
const step = (5 - 1) / 25;
//@ts-ignore
const data = runtime.dataSetsValues[1].data;
for (let i = 0; i < data.lenght; i++) {
const value = data.lenght;
const runtime = model.getters.getChartRuntime("1") as LineChartRuntime;
const step = (6 - 1) / 25;
const data = runtime.chartJsConfig.data.datasets[1].data;
for (let i = 0; i < data.length; i++) {
const value = data[i];
const expectedValue = Math.pow(1 + i * step, 2);
expect(value).toEqual(expectedValue);
expect(value).toBeCloseTo(expectedValue);
}
});

test("empty labels are correctly predicted", () => {
// prettier-ignore
setGrid(model, {
C6: "6",
C7: "7",
C8: "8",
C9: "9",
C10: "10",
B6: "", C6: "6",
B7: "", C7: "7",
B8: "", C8: "8",
B9: "", C9: "9",
B10: "", C10: "10",
});
updateChart(model, "1", {
dataSets: [{ dataRange: "B1:B10", trend: { display: true, type: "polynomial", order: 2 } }],
Expand All @@ -3179,14 +3178,44 @@ describe("trending line", () => {
offset: false,
labels: range(0, 51).map((v) => v.toString()),
});
const runtime = model.getters.getChartRuntime("1");
const step = (10 - 1) / 25;
const runtime = model.getters.getChartRuntime("1") as LineChartRuntime;
const step = (10 - 1) / 50;
const data = runtime.chartJsConfig.data.datasets[1].data;
for (let i = 0; i < data.length; i++) {
const value = data[i];
const expectedValue = Math.pow(1 + i * step, 2);
expect(value).toBeCloseTo(expectedValue);
}
});

test("trend line works with real date values as labels", () => {
setGrid(model, {
B1: "1",
C1: "1/7/2024",
B2: "4",
C2: "1/8/2024",
B3: "9",
C3: "1/9/2024",
B4: "16",
C4: "1/10/2024",
B5: "36",
C5: "1/12/2024",
});
const config = getChartConfiguration(model, "1");
expect(config.options.scales.x1).toMatchObject({
type: "category",
display: false,
offset: false,
labels: range(0, 26).map((v) => v.toString()),
});
const runtime = model.getters.getChartRuntime("1") as LineChartRuntime;
const step = (6 - 1) / 25;
//@ts-ignore
const data = runtime.dataSetsValues[1].data;
for (let i = 0; i < data.lenght; i++) {
const value = data.lenght;
const data = runtime.chartJsConfig.data.datasets[1].data;
for (let i = 0; i < data.length; i++) {
const value = data[i];
const expectedValue = Math.pow(1 + i * step, 2);
expect(value).toEqual(expectedValue);
expect(value).toBeCloseTo(expectedValue);
}
});

Expand Down

0 comments on commit d3a2181

Please sign in to comment.