Skip to content

feat: fill multi-series with missing x values data points#409

Merged
markov00 merged 2 commits intoelastic:masterfrom
markov00:add-missing-points
Oct 9, 2019
Merged

feat: fill multi-series with missing x values data points#409
markov00 merged 2 commits intoelastic:masterfrom
markov00:add-missing-points

Conversation

@markov00
Copy link
Copy Markdown
Collaborator

@markov00 markov00 commented Oct 7, 2019

Summary

fix #388

Before the fix

Screenshot 2019-10-07 at 22 36 10

After the fix:

Screenshot 2019-10-07 at 22 36 10

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

  • Any consumer-facing exports were added to src/index.ts (and stories only import from ../src except for test data & storybook)
  • This was checked for cross-browser compatibility, including a check against IE11
  • 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
  • Each commit follows the convention

@markov00 markov00 requested a review from nickofthyme October 7, 2019 20:40
@markov00 markov00 added :chart Chart element related issue :data Data/series/scales related issue labels Oct 7, 2019
@codecov-io
Copy link
Copy Markdown

codecov-io commented Oct 7, 2019

Codecov Report

Merging #409 into master will decrease coverage by 0.04%.
The diff coverage is 97.22%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #409      +/-   ##
==========================================
- Coverage      98%   97.95%   -0.05%     
==========================================
  Files          39       39              
  Lines        2853     2887      +34     
  Branches      681      692      +11     
==========================================
+ Hits         2796     2828      +32     
- Misses         50       52       +2     
  Partials        7        7
Impacted Files Coverage Δ
src/chart_types/xy_chart/rendering/rendering.ts 98.15% <100%> (ø) ⬆️
src/utils/commons.ts 94.73% <100%> (+0.29%) ⬆️
src/chart_types/xy_chart/store/utils.ts 96.76% <100%> (ø) ⬆️
src/chart_types/xy_chart/utils/series.ts 100% <100%> (ø) ⬆️
src/chart_types/xy_chart/domains/x_domain.ts 98.79% <66.66%> (-1.21%) ⬇️
...chart_types/xy_chart/utils/stacked_series_utils.ts 97.5% <98.11%> (-0.72%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 81bac09...64a3c9e. Read the comment docs.

When displaying a stacked area/line chart with multiple data sets we need to fill the dataset with
zeros on missing data points. The rendering is affected also: it will hide the filled missing point
but will continue to show the area beneath it.

fix elastic#388
@markov00 markov00 force-pushed the add-missing-points branch from 89c9c48 to 64a3c9e Compare October 8, 2019 17:25
@markov00 markov00 marked this pull request as ready for review October 8, 2019 17:41
@markov00 markov00 requested a review from rshen91 October 8, 2019 18:02
Copy link
Copy Markdown
Collaborator

@nickofthyme nickofthyme left a comment

Choose a reason for hiding this comment

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

Tested locally, LGTM.

This raises a lot of possible edge cases but I think you captured them all. At least that I can think of right now.

}

export interface DataSeriesDatum {
/** the x value */
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

😄

rawDataSeries: RawDataSeries[];
colorsValues: Map<string, any[]>;
xValues: Set<any>;
xValues: Set<string | number>;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nice I like the no any!

for (let x of missingXValues.values()) {
const stack = stackMap.get(x) || new Array(dataseries.length).fill(0);
// currently filling as 0 value
stack[index] = 0;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

👍

}
}
newData.sort((a, b) => {
if (xScaleType === ScaleType.Ordinal || typeof a.x === 'string' || typeof b.x === 'string') {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

👍

return legendValues.map((value, index) => {
const yAccessor: AccessorType = index === 0 ? AccessorType.Y1 : AccessorType.Y0;
return (
<LegendItem
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

👍 I missed that it was in a div 🤦‍♂

@markov00 markov00 merged commit ef84fd4 into elastic:master Oct 9, 2019
markov00 pushed a commit that referenced this pull request Oct 9, 2019
# [13.5.0](v13.4.1...v13.5.0) (2019-10-09)

### Features

* **data:** fill datasets with zeros with missing points when stacked ([#409](#409)) ([ef84fd4](ef84fd4)), closes [#388](#388)
@markov00
Copy link
Copy Markdown
Collaborator Author

markov00 commented Oct 9, 2019

🎉 This PR is included in version 13.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@markov00 markov00 added the released Issue released publicly label Oct 9, 2019
@markov00 markov00 deleted the add-missing-points branch November 25, 2020 11:43
AMoo-Miki pushed a commit to AMoo-Miki/OpenSearch-Dashboards that referenced this pull request Feb 10, 2022
# [13.5.0](elastic/elastic-charts@v13.4.1...v13.5.0) (2019-10-09)

### Features

* **data:** fill datasets with zeros with missing points when stacked ([opensearch-project#409](elastic/elastic-charts#409)) ([b989074](elastic/elastic-charts@b989074)), closes [opensearch-project#388](elastic/elastic-charts#388)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:chart Chart element related issue :data Data/series/scales related issue released Issue released publicly

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stacked area charts render with gaps when the X coordinates are not continuous

3 participants