-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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.
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
[NP] Inline buildPointSeriesData and buildHierarchicalData dependencies #61575
Changes from 12 commits
7fafc54
4cd75d1
2bc346b
ba83566
8283cad
45b33f8
6d3b22e
2020a63
9c2cc97
d05da68
8e39fbd
abcc032
f7609a4
4eb83d9
994b93e
207d32c
3de79f7
76f0266
5bd3a5d
f610ffd
15af087
da332e0
f6a4ed5
84fdace
bbc4624
e7c88f6
ad8ab46
10eca3f
a72aa56
1168932
acef340
b0d9e60
16028af
5617d0c
c0d6422
5f7b63e
d7e5043
838f5cf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
/* | ||
* Licensed to Elasticsearch B.V. under one or more contributor | ||
* license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright | ||
* ownership. Elasticsearch B.V. licenses this file to you under | ||
* the Apache License, Version 2.0 (the "License"); you may | ||
* not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
|
||
import { Table, Dimensions, Aspects } from './point_series'; | ||
|
||
/** | ||
* Identify and group the columns based on the aspect of the pointSeries | ||
* they represent. | ||
* | ||
* @return {object} - an object with a key for each aspect (see map). The values | ||
* may be undefined or an array of aspects. | ||
*/ | ||
export function getAspects(table: Table, dimensions: Dimensions) { | ||
const aspects: Aspects = {} as Aspects; | ||
(Object.keys(dimensions) as Array<keyof Dimensions>).forEach(name => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we know Discover's |
||
const d = dimensions[name]; | ||
const column = table.columns[d.accessor]; | ||
if (!column) { | ||
return; | ||
} | ||
aspects[name] = [ | ||
{ | ||
accessor: column.id, | ||
column: d.accessor, | ||
title: column.name, | ||
format: d.format, | ||
params: d.params, | ||
}, | ||
]; | ||
}); | ||
|
||
return aspects; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
/* | ||
* Licensed to Elasticsearch B.V. under one or more contributor | ||
* license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright | ||
* ownership. Elasticsearch B.V. licenses this file to you under | ||
* the Apache License, Version 2.0 (the "License"); you may | ||
* not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
|
||
import { Aspect, Table, Row } from './point_series'; | ||
|
||
type RowValue = number | 'NaN'; | ||
interface Raw { | ||
table: Table; | ||
column: number; | ||
row: number; | ||
value: RowValue; | ||
} | ||
export interface Point { | ||
x: RowValue; | ||
y: RowValue; | ||
extraMetrics: []; | ||
maryia-lapata marked this conversation as resolved.
Show resolved
Hide resolved
|
||
xRaw: Raw; | ||
yRaw: Raw; | ||
tableRaw?: { | ||
table: Table; | ||
column: number; | ||
row: number; | ||
value: number; | ||
title: string; | ||
}; | ||
parent: null; | ||
series?: string; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same for the If it's not used there, we can just throw it out. |
||
seriesId?: string; | ||
} | ||
|
||
export function getPoint(table: Table, x: Aspect, row: Row, rowIndex: number, y: Aspect) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems like the |
||
const xRow = row[x.accessor]; | ||
const yRow = row[y.accessor]; | ||
|
||
const point = { | ||
x: xRow, | ||
y: yRow, | ||
extraMetrics: [], | ||
xRaw: { | ||
table, | ||
column: x.column, | ||
row: rowIndex, | ||
value: xRow, | ||
}, | ||
yRaw: { | ||
table, | ||
column: y.column, | ||
row: rowIndex, | ||
value: yRow, | ||
}, | ||
tableRaw: table.$parent && { | ||
table: table.$parent.table, | ||
column: table.$parent.column, | ||
row: table.$parent.row, | ||
value: table.$parent.key, | ||
title: table.$parent.name, | ||
}, | ||
parent: null, | ||
} as Point; | ||
|
||
if (point.y === 'NaN') { | ||
// filter out NaN from stats | ||
// from metrics that are not based at zero | ||
return; | ||
} | ||
|
||
if (y) { | ||
// If the data is not split up with a series aspect, then | ||
// each point's "series" becomes the y-agg that produced it | ||
point.series = y.title; | ||
} | ||
|
||
return point; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
/* | ||
* Licensed to Elasticsearch B.V. under one or more contributor | ||
* license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright | ||
* ownership. Elasticsearch B.V. licenses this file to you under | ||
* the Apache License, Version 2.0 (the "License"); you may | ||
* not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
|
||
import _ from 'lodash'; | ||
import { getPoint, Point } from './_get_point'; | ||
import { addToSiri, Serie } from './_add_to_siri'; | ||
import { Chart, Table } from './point_series'; | ||
|
||
export function getSeries(table: Table, chart: Chart) { | ||
const aspects = chart.aspects; | ||
const xAspect = aspects.x[0]; | ||
const yAspect = aspects.y[0]; | ||
|
||
const partGetPoint = _.partial(getPoint, table, xAspect); | ||
|
||
return _(table.rows) | ||
.transform((series: any, row, rowIndex) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thinking about it even further - I think we can completely remove the logic around "series" and "points". In the special case of discover we are just copying over x and y values, so we can just use the table as it is, no need for all of these abstraction layers. |
||
const point: Point = partGetPoint(row, rowIndex, yAspect); | ||
if (point) { | ||
const id = `${point.series}-${yAspect.accessor}`; | ||
point.seriesId = id; | ||
addToSiri(series as Map<string, Serie>, point, id); | ||
} | ||
}, new Map<string, Serie>() as any) | ||
.thru(series => [...series.values()]) | ||
.value() as Serie[]; | ||
} |
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -0,0 +1,133 @@ | ||||
/* | ||||
* Licensed to Elasticsearch B.V. under one or more contributor | ||||
* license agreements. See the NOTICE file distributed with | ||||
* this work for additional information regarding copyright | ||||
* ownership. Elasticsearch B.V. licenses this file to you under | ||||
* the Apache License, Version 2.0 (the "License"); you may | ||||
* not use this file except in compliance with the License. | ||||
* You may obtain a copy of the License at | ||||
* | ||||
* http://www.apache.org/licenses/LICENSE-2.0 | ||||
* | ||||
* Unless required by applicable law or agreed to in writing, | ||||
* software distributed under the License is distributed on an | ||||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||||
* KIND, either express or implied. See the License for the | ||||
* specific language governing permissions and limitations | ||||
* under the License. | ||||
*/ | ||||
|
||||
import { uniq } from 'lodash'; | ||||
import moment, { Duration } from 'moment'; | ||||
import { Unit } from '@elastic/datemath'; | ||||
|
||||
import { SerializedFieldFormat } from '../../../../../../../../plugins/expressions/common/types'; | ||||
import { getSeries } from './_get_series'; | ||||
import { getAspects } from './_get_aspects'; | ||||
import { Serie } from './_add_to_siri'; | ||||
|
||||
export interface Column { | ||||
id: string; | ||||
name: string; | ||||
} | ||||
|
||||
export interface Row { | ||||
[key: string]: number; | ||||
} | ||||
|
||||
export interface Table { | ||||
columns: Column[]; | ||||
rows: Row[]; | ||||
$parent: { | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like we never have the |
||||
table: Table; | ||||
column: number; | ||||
row: number; | ||||
key: number; | ||||
name: string; | ||||
}; | ||||
} | ||||
|
||||
interface HistogramParams { | ||||
date: true; | ||||
interval: Duration; | ||||
intervalESValue: number; | ||||
intervalESUnit: Unit; | ||||
format: string; | ||||
bounds?: { | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't kibana/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js Line 858 in c0a14cd
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems so. Please take a look at 10eca3f. |
||||
min: string | number; | ||||
max: string | number; | ||||
}; | ||||
} | ||||
export interface Dimension { | ||||
accessor: 0 | 1; | ||||
format: SerializedFieldFormat<{ pattern: string }>; | ||||
params?: HistogramParams; | ||||
} | ||||
|
||||
export interface Dimensions { | ||||
x: Dimension; | ||||
y: Dimension; | ||||
} | ||||
export interface Aspect { | ||||
accessor: Column['id']; | ||||
column?: Dimension['accessor']; | ||||
title: Column['name']; | ||||
format: Dimension['format']; | ||||
params: Dimension['params']; | ||||
} | ||||
export interface Aspects { | ||||
x: Aspect[]; | ||||
y: Aspect[]; | ||||
} | ||||
interface Ordered { | ||||
date: true; | ||||
interval: Duration | number; | ||||
intervalESUnit: string; | ||||
intervalESValue: number; | ||||
min?: number; | ||||
max?: number; | ||||
} | ||||
export interface Chart { | ||||
aspects: Aspects; | ||||
series: Serie[]; | ||||
xAxisOrderedValues: number[]; | ||||
xAxisFormat: Dimension['format']; | ||||
xAxisLabel: Column['name']; | ||||
yAxisLabel?: Column['name']; | ||||
ordered: Ordered; | ||||
} | ||||
|
||||
export const buildPointSeriesData = (table: Table, dimensions: Dimensions) => { | ||||
const chart: Chart = { | ||||
aspects: getAspects(table, dimensions), | ||||
} as Chart; | ||||
|
||||
const { format, title, params, accessor } = chart.aspects.x[0]; | ||||
chart.xAxisOrderedValues = uniq(table.rows.map(r => r[accessor])); | ||||
chart.xAxisFormat = format; | ||||
chart.xAxisLabel = title; | ||||
|
||||
const { intervalESUnit, intervalESValue, interval, bounds } = params as HistogramParams; | ||||
chart.ordered = { | ||||
date: true, | ||||
interval: moment.duration(interval), | ||||
intervalESUnit, | ||||
intervalESValue, | ||||
}; | ||||
|
||||
if (bounds) { | ||||
chart.ordered.min = isNaN(bounds.min as number) | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the types from the comments above are fixed, then these type casts should go away. |
||||
? Date.parse(bounds.min as string) | ||||
: (bounds.min as number); | ||||
chart.ordered.max = isNaN(bounds.max as number) | ||||
? Date.parse(bounds.max as string) | ||||
: (bounds.max as number); | ||||
} | ||||
|
||||
chart.yAxisLabel = chart.aspects.y && chart.aspects.y[0].title; | ||||
|
||||
chart.series = getSeries(table, chart); | ||||
|
||||
delete chart.aspects; | ||||
return chart; | ||||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the component the
values
is used only, so I removed all other fields.