feat: auto interval approximation #1294

nickofthyme · 2021-08-10T18:53:23Z

Summary

The PR adds auto approximation of the minInterval when not directly inferable. This includes both time and non-time based data.

Details

This PR adds a simplified approximation of the minInterval for single point datasets when a min and max xDomain is provided without a minInterval. Currently, we default the interval in such cases to 1. This is problematic as this often prohibits rendering the bar given its infinitesimal width.

The solution in this case is to determine the best unit interval that would reasonably display the single data point. This is done by looping through different time units of decreasing scale to determine the best time unit for the provided domain extents. Some examples include:

domain extents	approx. minInterval
40 years	1 year
10 years	1 year
52 weeks	1 week
4 weeks	1 week
1 week	1 day
8 days	1 day
7 days	1 day
1 days	1 hour
24 hours	1 hour
1 hour	1 minute
60 minutes	1 minute

Notice all approximate minIntervals are a single unit of time. It has been noted as a future enhancement to this logic to account for multi-factor units such as 12 hours, to best fit the given data.

A development warning is surfaced in the console to promote adding the actual minInterval based on the data.

Issues

fixes #840, fixes #1184

Checklist

The proper chart type label was added (e.g. :xy, :partition) if the PR involves a specific chart type
The proper feature label was added (e.g. :interactions, :axis) if the PR involves a specific chart feature
Whenever possible, please check if the closing issue is connected to a running GH project
Proper documentation or storybook story was added for features that require explanation or tutorials
Unit tests were updated or added to match the most common scenarios

markov00 · 2021-08-11T17:23:29Z

packages/charts/src/chart_types/xy_chart/domains/x_domain.ts

+
+  for (let i = 0; i < approximatedOptions.length; i++) {
+    const { unit, min = 1, max = 10 } = approximatedOptions[i];
+    const unitInterval = moment.duration(1, unit).asMilliseconds();


can we just add the millisecond resolution in the approximatedOptions?

Not sure what you are suggesting here. Do you want me to remove the milliseconds option altogether?

markov00 · 2021-08-11T17:24:44Z

packages/charts/src/chart_types/xy_chart/domains/x_domain.ts

  const valuesLength = xValues.length;
  if (valuesLength <= 0) {
    return 0;
  }
  if (valuesLength === 1) {
-    return 1;
+    return null;


Instead of returning null, can we just call guessMinInterval(seriesXComputedDomains, fallbackScale ?? type); here?

Suggested change

return null;

return guessMinInterval(seriesXComputedDomains, fallbackScale ?? type);

I thought about that but then I would need to add two more arguments to this function. Which would you prefer?

fwiw I tend to trade argument list brevity for return type simplicity so I wouldn't mind having two extra args, although there may be some better solution. Infinity would make it number consistently—see the minDelta suggestion below—, and then it needs a Number.isFinite check, or some other way of handling the infinity

Putting aside the question around null the function is basically a reduction:

export function findMinInterval(xValues: number[]): number | null { const sortedValues = xValues.slice().sort(compareByValueAsc); return xValues.length === 0 ? 0 : xValues.length === 1 ? null : sortedValues.reduce( (p, n, i, a) => Math.min(p, (i < a.length - 1 ? a[i + 1] : Infinity) - n), sortedValues[1] - sortedValues[0], ); }

Somehow it feels asymmetrical that for a zero length array, we return zero, and for a 1-length array, it's null, then from 2, it's a number again

Oh my point about this being a reduction: probably it belongs in some basic array processing util, not x_domain.ts, so

export function findMinInterval(xValues: number[]): number | null { const sortedValues = xValues.slice().sort(compareByValueAsc); return xValues.length === 0 ? 0 : xValues.length === 1 ? null : minDelta(sortedValues) }

Or minDelta does the sorting too:

const minDelta = (values: number, predicate: Sorter): number => values .slice() .sort(predicate) .reduce((p, n, i, a) => Math.min(p, (i < a.length - 1 ? a[i + 1] : Infinity) - n), Infinity);

It naturally yields Infinity if there element count is less than 2, so maybe that could be used instead of null

monfera · 2021-08-18T13:38:52Z

packages/charts/src/chart_types/xy_chart/domains/x_domain.ts

@@ -127,18 +133,77 @@ function getMinInterval(computedMinInterval: number, size: number, customMinInte
  return customMinInterval;
 }

+const approximatedOptions: {


Would there be a more telling name for anything in the code change with approx* in it, as what is going on here doesn't look like approximation, more like, inferred min interval? Yeah any min interval is inferred that's not expressly given, so if it's ambiguous, it could be more explicit, like singleton min interval or some such, easy to grep.

The preexisting minInterval itself is already a bit of a misnomer, esp. for the case of a singleton bar, when there's no interval by definition. I see such alternatives in dataviz land as

pitch, temporal pitch

cadence

bin width, temporal bin width

So it might be binWidthFromSinglePoint or binCountForSinglePoint even

monfera · 2021-08-18T13:46:11Z

packages/charts/src/chart_types/xy_chart/domains/x_domain.ts

+  { unit: 'second', max: 60 },
+  { unit: 'millisecond', max: 1000 },


It means that if the specified time domain is such that there'd be 60 seconds in the domain, then it'll accept minute? So the entire chart width becomes one giant bar.

What drives the numbers?

Asking it as we already have problems with enormous bar widths that no dataviz designer would manually consider. I realize it's an edge case, but a minute-bar shown on a 61-second domain will be almost full Cartesian area width, or if there's some nicing of the time axis(?) 1/3rd to half of the Cartesian width.

Also, shouldn't the whole thing depend on the screenspace Cartesian width ie. pixels, or estimated bar width pixels or bar pitch pixels? It feels like showing a minute-bar in a 61-second domain is great if the chart is very narrow, but not so great if it spans a dashboard horizontally

monfera · 2021-08-18T13:53:47Z

packages/charts/src/chart_types/xy_chart/domains/x_domain.ts

+  Logger.warn(
+    `The minInterval, given only a single data point, has been approximated as ${
+      scaleType === ScaleType.Time ? moment.duration(value).humanize() : value
+    }. This is not ideal as this data point likely does not coincide with the proper interval. Please set the minInterval in the xDomain.`,


Nit: the linter complains of line length but it's a template string, what do we want to generally do in cases like this?

monfera · 2021-08-18T13:57:30Z

packages/charts/src/chart_types/xy_chart/domains/x_domain.ts

  const valuesLength = xValues.length;
  if (valuesLength <= 0) {
    return 0;
  }
  if (valuesLength === 1) {
-    return 1;
+    return null;
  }
  const sortedValues = xValues.slice().sort(compareByValueAsc);
  let i;


Might as well put the let inside the for

monfera · 2021-08-18T14:04:23Z

packages/charts/src/chart_types/xy_chart/domains/x_domain.ts

 * @internal
 */
-export function findMinInterval(xValues: number[]): number {
+export function findMinInterval(xValues: number[]): number | null {
  const valuesLength = xValues.length;
  if (valuesLength <= 0) {


Preexisting: [].length can't be less than 0 (I'm OK with a <= convention though if we want to save on bytes)

monfera · 2021-08-18T14:09:30Z

packages/charts/src/chart_types/xy_chart/domains/x_domain.ts

  const valuesLength = xValues.length;
  if (valuesLength <= 0) {
    return 0;
  }
  if (valuesLength === 1) {
-    return 1;
+    return null;
  }
  const sortedValues = xValues.slice().sort(compareByValueAsc);


Since we sort, there's no need for the Math.abs in const interval = Math.abs(next - current);

monfera · 2021-08-18T15:26:56Z

packages/charts/src/chart_types/xy_chart/domains/x_domain.ts

+  min?: number;
+  max?: number;
+}[] = [
+  { unit: 'year' },


{ unit: 'year' , max: Infinity}

monfera · 2021-08-18T15:27:36Z

packages/charts/src/chart_types/xy_chart/domains/x_domain.ts

+    const unitInterval = moment.duration(1, unit).asMilliseconds();
+    const unitDiff = Math.floor(value / unitInterval);
+
+    if ((unitDiff > min && unitDiff <= max) || (unit === 'year' && unitDiff > max)) {


... then no need for the extra || (unit === 'year' && unitDiff > max) check

Minor: it's also fine, and maybe more intuitive here to permit the min value too, and using a math-like ordering min <= unitDiff <= max

min <= unitDiff && unitDiff <= max

nickofthyme · 2021-08-30T14:07:22Z

Closing in favor of different approach, that being to require min-interval on BarSeriesSpec and remove from xDomain. Keep comments for future consideration.

feat: auto interval approximation

828ed7a

nickofthyme added enhancement New feature or request :data Data/series/scales related issue :xy Bar/Line/Area chart related labels Aug 10, 2021

nickofthyme requested review from markov00, monfera and rshen91 August 10, 2021 18:53

nickofthyme added 5 commits August 10, 2021 14:24

chore: validate fix for heatmap issue elastic#1184

a67fdc6

fix: single values with zero domain extents

979eacb

test: update test constant expect

1b07e47

fix: warning message

84a7f28

Merge branch 'master' into fix-single-time-value

857ce54

nickofthyme closed this Aug 11, 2021

nickofthyme reopened this Aug 11, 2021

elastic deleted a comment from github-actions bot Aug 11, 2021

markov00 reviewed Aug 11, 2021

View reviewed changes

monfera reviewed Aug 18, 2021

View reviewed changes

nickofthyme closed this Aug 30, 2021

nickofthyme deleted the fix-single-time-value branch August 30, 2021 14:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: auto interval approximation #1294

feat: auto interval approximation #1294

nickofthyme commented Aug 10, 2021 •

edited

Loading

markov00 Aug 11, 2021

nickofthyme Aug 17, 2021

markov00 Aug 11, 2021

nickofthyme Aug 17, 2021

monfera Aug 18, 2021 •

edited

Loading

monfera Aug 18, 2021 •

edited

Loading

monfera Aug 18, 2021 •

edited

Loading

monfera Aug 18, 2021 •

edited

Loading

monfera Aug 18, 2021 •

edited

Loading

monfera Aug 18, 2021

monfera Aug 18, 2021

monfera Aug 18, 2021

monfera Aug 18, 2021

monfera Aug 18, 2021

monfera Aug 18, 2021

monfera Aug 18, 2021

nickofthyme commented Aug 30, 2021

	return null;
	return guessMinInterval(seriesXComputedDomains, fallbackScale ?? type);

		{ unit: 'second', max: 60 },
		{ unit: 'millisecond', max: 1000 },

feat: auto interval approximation #1294

feat: auto interval approximation #1294

Conversation

nickofthyme commented Aug 10, 2021 • edited Loading

Summary

Details

Issues

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monfera Aug 18, 2021 • edited Loading

Choose a reason for hiding this comment

monfera Aug 18, 2021 • edited Loading

Choose a reason for hiding this comment

monfera Aug 18, 2021 • edited Loading

Choose a reason for hiding this comment

monfera Aug 18, 2021 • edited Loading

Choose a reason for hiding this comment

monfera Aug 18, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nickofthyme commented Aug 30, 2021

nickofthyme commented Aug 10, 2021 •

edited

Loading

monfera Aug 18, 2021 •

edited

Loading

monfera Aug 18, 2021 •

edited

Loading

monfera Aug 18, 2021 •

edited

Loading

monfera Aug 18, 2021 •

edited

Loading

monfera Aug 18, 2021 •

edited

Loading