-
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 26 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,125 @@ | ||||
/* | ||||
* 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'; | ||||
|
||||
export interface Column { | ||||
id: string; | ||||
name: string; | ||||
} | ||||
|
||||
export interface Row { | ||||
[key: string]: number | 'NaN'; | ||||
} | ||||
|
||||
export interface Table { | ||||
columns: Column[]; | ||||
rows: Row[]; | ||||
} | ||||
|
||||
interface HistogramParams { | ||||
date: true; | ||||
interval: Duration; | ||||
intervalESValue: number; | ||||
intervalESUnit: Unit; | ||||
format: string; | ||||
bounds?: { | ||||
min: string | number; | ||||
max: string | number; | ||||
}; | ||||
} | ||||
export interface Dimension { | ||||
accessor: 0 | 1; | ||||
format: SerializedFieldFormat<{ pattern: string }>; | ||||
} | ||||
|
||||
export interface Dimensions { | ||||
x: Dimension & { params: HistogramParams }; | ||||
y: Dimension; | ||||
} | ||||
|
||||
interface Ordered { | ||||
date: true; | ||||
interval: Duration | number; | ||||
intervalESUnit: string; | ||||
intervalESValue: number; | ||||
min?: number; | ||||
max?: number; | ||||
} | ||||
export interface Chart { | ||||
values: Array<{ | ||||
x: number; | ||||
y: number; | ||||
}>; | ||||
xAxisOrderedValues: number[]; | ||||
xAxisFormat: Dimension['format']; | ||||
xAxisLabel: Column['name']; | ||||
yAxisLabel?: Column['name']; | ||||
ordered: Ordered; | ||||
} | ||||
|
||||
export const buildPointSeriesData = (table: Table, dimensions: Dimensions) => { | ||||
const { x, y } = dimensions; | ||||
const xAccessor = table.columns[x.accessor].id; | ||||
const yAccessor = table.columns[y.accessor].id; | ||||
const chart = {} as Chart; | ||||
|
||||
chart.xAxisOrderedValues = uniq(table.rows.map(r => r[xAccessor] as number)); | ||||
chart.xAxisFormat = x.format; | ||||
chart.xAxisLabel = table.columns[x.accessor].name; | ||||
|
||||
const { intervalESUnit, intervalESValue, interval, bounds } = x.params; | ||||
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 = table.columns[y.accessor].name; | ||||
|
||||
chart.values = []; | ||||
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. 2 nits: I am not sure why you're explicitly passing
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. Nice catch! thanks, I updated the PR. |
||||
table.rows.forEach((row, rowIndex) => { | ||||
if (row && row[yAccessor] !== 'NaN') { | ||||
chart.values.push({ | ||||
x: row[xAccessor] as number, | ||||
y: row[yAccessor] as number, | ||||
}); | ||||
} | ||||
}); | ||||
|
||||
if (!chart.values.length) { | ||||
chart.values.push({} as any); | ||||
} | ||||
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. I think you can easily remove this lines, by a modification of histogram.tsx kibana/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/directives/histogram.tsx Line 231 in 82e048a
e.g. by using
you no longer need to push this empty object, and the histogram is displaying 'No data to display' 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. thanks, done. |
||||
|
||||
return chart; | ||||
}; |
This file was deleted.
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.
Isn't
bounds
always set in discover when there is a histogram?Also, it seems like the actual type of this is a moment instance:
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 comment
The reason will be displayed to describe this comment to others. Learn more.
It seems so. Please take a look at 10eca3f.