[Lens] Calculation operations #83789

flash1293 · 2020-11-19T12:45:42Z

Based on #83603

This PR adds some real implementations for reference-based operations:

Cumulative sum
Derivative
Moving average
Counter rate

cc @wylieconlon

…tion

flash1293 · 2020-11-20T16:16:56Z

@wylieconlon This should build fine now

wylieconlon

This is looking good! I left some stylistic comments, but this will definitely help develop the UI :)

...ins/lens/public/indexpattern_datasource/operations/definitions/calculations/counter_rate.tsx

...ugins/lens/public/indexpattern_datasource/operations/definitions/calculations/derivative.tsx

wylieconlon · 2020-11-20T20:14:21Z

...s/lens/public/indexpattern_datasource/operations/definitions/calculations/moving_average.tsx

+        defaultMessage: 'Window size',
+      })}
+      display="columnCompressed"
+      fullWidth


This form input is a good candidate for help text. I would like to explain some of the comments that we've written in the definition of the window, such as how it's trailing and how it deals with missing values.

Do we have a design for that yet?

There is no final design, but the idea that @MichaelMarcialis wants to explore is a popover-based help text. Tracking that effort here: #81780

wylieconlon · 2020-11-20T20:22:17Z

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/utils.ts

+  }
+  return [
+    i18n.translate('xpack.lens.indexPattern.calculations.dateHistogramErrorMessage', {
+      defaultMessage: 'Needs a date histogram to work',


I think this error message would be improved by adding more context, like Cumulative sum requires a date histogram to work. Choose a different function or add a date histogram. This context might need to be passed in as an argument to the function

wylieconlon · 2020-11-20T20:23:48Z

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/utils.ts

+  additionalArgs: Record<string, unknown[]> = {}
+): ExpressionFunctionAST[] {
+  const currentColumn = (layer.columns[columnId] as unknown) as ReferenceBasedIndexPatternColumn;
+  const buckets = layer.columnOrder.filter((colId) => layer.columns[colId].isBucketed);


Small tweak to the code style:

Suggested change

const buckets = layer.columnOrder.filter((colId) => layer.columns[colId].isBucketed);

const buckets = layer.columnOrder.filter((colId) => layer.columns[colId].isBucketed && layer.columns[colId].operationType !== 'date_histogram');

That's not just a code style change - if there are two date histogram columns in the current table, this will prevent grouping by the second for applying the calculation, producing meaningless data.

Good point, hadn't realized.

wylieconlon · 2020-11-20T20:24:12Z

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/utils.ts

+  const dateColumnIndex = buckets.findIndex(
+    (colId) => layer.columns[colId].operationType === 'date_histogram'
+  )!;
+  buckets.splice(dateColumnIndex, 1);


Suggested change

const dateColumnIndex = buckets.findIndex(

(colId) => layer.columns[colId].operationType === 'date_histogram'

)!;

buckets.splice(dateColumnIndex, 1);

const dateColumnIndex = layer.columnOrder.findIndex(

(colId) => layer.columns[colId].operationType === 'date_histogram'

)!;

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/utils.ts

…tion

flash1293 · 2020-11-23T13:30:58Z

Thanks for the review @wylieconlon , I worked through the comments you didn't tackle yourself. There's a code style recommendation I don't get, could you check out that one?

flash1293 · 2020-11-23T13:33:37Z

@elasticmachine merge upstream

elasticmachine · 2020-11-23T16:22:15Z

Pinging @elastic/kibana-app (Team:KibanaApp)

wylieconlon · 2020-11-23T19:28:12Z

@flash1293 I know this isn't related to this code directly, but it looks like the lens_counter_rate expression function isn't actually registered to the expression plugin. Do you want to add that as part of this PR or separately?

wylieconlon · 2020-11-23T23:49:33Z

...s/lens/public/indexpattern_datasource/operations/definitions/calculations/cumulative_sum.tsx

+
+const ofName = (name?: string) => {
+  return i18n.translate('xpack.lens.indexPattern.cumulativeSumOf', {
+    defaultMessage: 'Cumulative sum rate of {name}',


Are we expecting this to be Cumulative sum of <field>? It's currently Cumulative sum of <reference label>?

This is part of my reference UI work, so you might not need to make a change. I'm regenerating the default labels whenever you modify an operation.

hm, good point, I'm not sure actually. Let's keep that for now and see whether we want to keep it once transition/UI is in place

…tion

flash1293 · 2020-11-24T14:53:45Z

@wylieconlon Registered counter rate as part of this PR

kibanamachine · 2020-11-24T17:02:49Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 3f9fb7d

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`lens`	438	445	+7

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`lens`	936.9KB	952.6KB	+15.6KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`lens`	52.8KB	52.9KB	+93.0B

History

💚 Build #89604 succeeded 0271146
💚 Build #89403 succeeded c208fde
💚 Build #89382 succeeded 26db7b8
💚 Build #89363 succeeded c0a16e4
💚 Build #89284 succeeded cc902e2

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

# Conflicts: # x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.test.ts # x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.ts

wylieconlon and others added 8 commits November 17, 2020 18:55

[Lens] Implement types for reference-based operations

eb4b573

Merge remote-tracking branch 'origin/master' into lens/reference-types

b4fb6ea

Merge remote-tracking branch 'origin/master' into lens/reference-types

6024a31

Update from review feedback

104595f

wip

963cfc1

Merge remote-tracking branch 'upstream/master' into lens/cumsum_opera…

6cea768

…tion

fix problem

ad6f522

fix types

1c1f69d

flash1293 changed the title ~~[Lens] Cumulative sum operation~~ [Lens] Calculation operations Nov 20, 2020

flash1293 added 4 commits November 20, 2020 13:57

Merge remote-tracking branch 'upstream/master' into lens/cumsum_opera…

cf94d0c

…tion

add other operations

7ef2558

add window param

724e7c3

fix circular import

cc902e2

Merge remote-tracking branch 'origin/master' into HEAD

c0a16e4

wylieconlon approved these changes Nov 20, 2020

View reviewed changes

wylieconlon and others added 4 commits November 20, 2020 15:26

Change types in test

26db7b8

Fix labeling

c208fde

Merge remote-tracking branch 'upstream/master' into lens/cumsum_opera…

1e42fcd

…tion

fix stuff

4751e99

Merge branch 'master' into lens/cumsum_operation

0271146

flash1293 marked this pull request as ready for review November 23, 2020 16:21

flash1293 requested a review from a team November 23, 2020 16:21

flash1293 added Feature:Lens release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.11.0 v8.0.0 labels Nov 23, 2020

wylieconlon reviewed Nov 23, 2020

View reviewed changes

flash1293 added 2 commits November 24, 2020 15:47

Merge remote-tracking branch 'upstream/master' into lens/cumsum_opera…

620f095

…tion

register counter rate function

3f9fb7d

flash1293 merged commit a763d33 into elastic:master Nov 25, 2020

flash1293 mentioned this pull request Nov 25, 2020

[7.x] [Lens] Calculation operations (#83789) #84305

Closed

flash1293 added a commit to flash1293/kibana that referenced this pull request Nov 26, 2020

[Lens] Calculation operations (elastic#83789)

3c3809a

flash1293 mentioned this pull request Nov 26, 2020

[7.x] [Lens] Calculation operations (#83789) #84390

Merged

flash1293 added a commit that referenced this pull request Nov 26, 2020

[Lens] Calculation operations (#83789) (#84390)

86bb03d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Lens] Calculation operations #83789

[Lens] Calculation operations #83789

flash1293 commented Nov 19, 2020 •

edited

Loading

flash1293 commented Nov 20, 2020

wylieconlon left a comment

wylieconlon Nov 20, 2020

flash1293 Nov 23, 2020

wylieconlon Nov 23, 2020

wylieconlon Nov 20, 2020

wylieconlon Nov 20, 2020

flash1293 Nov 23, 2020

wylieconlon Nov 23, 2020

wylieconlon Nov 20, 2020

flash1293 commented Nov 23, 2020

flash1293 commented Nov 23, 2020

elasticmachine commented Nov 23, 2020

wylieconlon commented Nov 23, 2020

wylieconlon Nov 23, 2020

flash1293 Nov 24, 2020

flash1293 commented Nov 24, 2020

kibanamachine commented Nov 24, 2020

	const buckets = layer.columnOrder.filter((colId) => layer.columns[colId].isBucketed);
	const buckets = layer.columnOrder.filter((colId) => layer.columns[colId].isBucketed && layer.columns[colId].operationType !== 'date_histogram');

[Lens] Calculation operations #83789

[Lens] Calculation operations #83789

Conversation

flash1293 commented Nov 19, 2020 • edited Loading

flash1293 commented Nov 20, 2020

wylieconlon left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flash1293 commented Nov 23, 2020

flash1293 commented Nov 23, 2020

elasticmachine commented Nov 23, 2020

wylieconlon commented Nov 23, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flash1293 commented Nov 24, 2020

kibanamachine commented Nov 24, 2020

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

History

flash1293 commented Nov 19, 2020 •

edited

Loading