Skip to content
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.

Sign up for GitHub

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

Fix rescaling y-axis edge cases #1437

Merged
merged 1 commit into from
Jun 22, 2021
Merged

Fix rescaling y-axis edge cases #1437

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 28 additions & 10 deletions src/components/explore/AggregationsOverTimeGraph.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,17 +144,25 @@
$showContextMenu = true; // eslint-disable-line no-unused-vars
}

const linearMetrics = [
'histogram-linear',
'quantity',
'counter',
'labeled_counter',
];
Comment on lines +147 to +152
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These metrics have linear graph type, but initially I only had histogram-linear and quantity on this list, so other linear graphs such as counter and labeled_counter did not scale (see counter and labeled_counter)

After adding them to the list, we can rescale y-axis for counter and labeled_counter now.

Working example of a counter metric:
2021-06-22 12 28 21


const getYTicks = (ranges) => {
// exponential and linear graphs
if (
data[0].metric_type === 'histogram-linear' ||
data[0].metric_type === 'quantity' ||
linearMetrics.includes(data[0].metric_type) ||
yScaleType === 'scalePoint'
) {
// when the range is too small we need to custom set it
if (ranges[ranges.length - 1] <= 5) return [0, 1, 2, 3, 4, 5];
if (yScaleType === 'scalePoint' && ranges.length < 5) return yValues;
return undefined;
}
// categorical graphs
if (
$store.activeBuckets.length &&
$store.proportionMetricType === 'proportions'
Expand All @@ -178,20 +186,32 @@

// get percentile data of linear and log graphs
if (
data[0].metric_type === 'histogram-linear' ||
data[0].metric_type === 'quantity' ||
linearMetrics.includes(data[0].metric_type) ||
yScaleType === 'scalePoint'
) {
// do not change yDomain when all percentiles are selected
// (we want the graph to match the violin plot initially)
if ($store.visiblePercentiles.length === 5) return yDomain;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were not able to rescale the y axis because we used to have the same set of data for yDomain regardless of toggled percentiles. In my last change, I calculated the yDomain range to only include the highest and smallest values available of the shown percentile lines, so that this array will dynamically change if a percentile is toggled. However this means that the range is more tightly scoped than before, and you can see the difference in initial behavior here: dev vs stage.

@rafrombrc wanted the initial range of the graph to match the violin plot graph, so I added this condition here to preserve the yDomain range if all percentile lines are visible (i.e no percentiles toggled yet)

Below you can see that the yDomain range matches the violin plot initially.

match

visiblePercentiles.forEach((p) => {
yData = yData.concat([...data.map((arr) => arr.percentiles[p])]);
});
// use transformed data for Android metrics
if (data[0].transformedPercentiles) {
visiblePercentiles.forEach((p) => {
yData = yData.concat([
...data.map((arr) => arr.transformedPercentiles[p]),
]);
});
}
Comment on lines +199 to +205
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently on dev this metric doesn't have any data. After changing data source to $store.transformedPercentiles (which I believe is the main source of data for Glean metrics, we use $store.percentiles for desktop probes), it does now.

2021-06-22 12 03 58

}
// get proportion and count data of categorical graphs
if (
buckets.length &&
data[0].metric_type !== 'histogram-linear' &&
yScaleType !== 'scalePoint'
) {
// do not change yDomain when all categories are selected
if ($store.activeBuckets.length === 10) return yDomain;
if ($store.proportionMetricType === 'proportions') {
buckets.forEach((bucket) => {
yData = yData.concat([...data.map((arr) => arr.proportions[bucket])]);
Expand All @@ -205,14 +225,12 @@
}

yDomainValues = _.uniq(yData).sort((a, b) => a - b);
yDomainValues = yDomainValues.filter((a) => !Number.isNaN(a));
yDomainValues = yDomainValues.filter(
(a) => !Number.isNaN(a) && Number.isFinite(a)
);

// set the range for each graph type based on graph-paper setting
if (
yScaleType === 'linear' &&
(data[0].metric_type === 'histogram-linear' ||
data[0].metric_type === 'quantity')
)
if (yScaleType === 'linear' && linearMetrics.includes(data[0].metric_type))
return yDomainValues[yDomainValues.length - 1]
? [yDomainValues[0], yDomainValues[yDomainValues.length - 1]]
: [0, 1];
Expand Down