feat(series): stack series in percentage mode by markov00 · Pull Request #250 · elastic/elastic-charts

markov00 · 2019-06-26T16:21:51Z

Summary

This PR will add a new props that allows stacking bars, areas and lines using the percentage
mode. For each stacked x value, we scale their value to the percentage on that bucket.

That option mode is applied by groups of series (specified by groupId) so you need to specify only one prop on one of the group series the property: stackAsPercentage={true}

<Axis 
  id={getAxisId('count')} 
  title="count" 
  position={Position.Left} 
  tickFormat={d => `${Number(d * 100).toFixed(1)}%`}/>
<AreaSeries
    id={getSpecId('dataset A')}
    xScaleType={ScaleType.Time}
    yScaleType={ScaleType.Linear}
    data={KIBANA_METRICS.metrics.kibana_os_load[1].data}
    xAccessor={0}
    yAccessors={[1]}
    stackAccessors={[0]}
    stackAsPercentage={true}
  />
<AreaSeries
    id={getSpecId('dataset B')}
    xScaleType={ScaleType.Time}
    yScaleType={ScaleType.Linear}
    data={KIBANA_METRICS.metrics.kibana_os_load[0].data}
    xAccessor={0}
    yAccessors={[1]}
    stackAccessors={[0]}
    stackAsPercentage={true}
  />

100% stacked bars:

100% stacked areas:

100% stacked areas with y0accessors on a single area:

100% stacked areas with y0accessors on multiple areas:

fix #222

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

This commit will add a new props that allows stacking bars, areas and lines using the percentage mode. For each stacked x value, we scale their value to the percentage on that bucket. fix elastic#222

codecov-io · 2019-06-26T16:28:45Z

Codecov Report

Merging #250 into master will increase coverage by 0.21%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #250      +/-   ##
==========================================
+ Coverage   97.76%   97.97%   +0.21%     
==========================================
  Files          36       36              
  Lines        2641     2920     +279     
  Branches      590      702     +112     
==========================================
+ Hits         2582     2861     +279     
+ Misses         52       50       -2     
- Partials        7        9       +2

Impacted Files	Coverage Δ
src/lib/series/specs.ts	`100% <ø> (ø)`	⬆️
src/state/utils.ts	`96.44% <100%> (+0.2%)`	⬆️
src/lib/series/series.ts	`100% <100%> (ø)`	⬆️
src/lib/series/domains/y_domain.ts	`100% <100%> (ø)`	⬆️
src/lib/series/stacked_series_utils.ts	`98.21% <100%> (+2.65%)`	⬆️
src/lib/series/legend.ts	`100% <100%> (ø)`	⬆️
src/lib/axes/canvas_text_bbox_calculator.ts	`100% <0%> (ø)`	⬆️
src/lib/axes/axis_utils.ts	`100% <0%> (ø)`	⬆️
src/lib/utils/commons.ts	`100% <0%> (ø)`	⬆️
... and 1 more

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 fc6640d...3093ec9. Read the comment docs.

emmacunningham

Left a few inline code comments, some other things that I've noticed:

last bucket value in legend is not visible in legend when bars are clustered

The current hover value is visible, but not the default last value.

multiple series
We should be careful to document this for the consuming users so that they understand that for stackAsPercentage to work correctly, the need to define stackAccessors for each series or else the chart will look quite strange. Below is what the chart looks like if you enable stackAsPercentage on one series but only define stackAccessors for one series:

Can we also add a story for the area series?

Also, should stackAsPercentage be unavailable as a prop on a line series? It seems to me that without the context that is provided by the area series, this prop with a line series could present very misleading data to the point that we shouldn't expose it as a possibility for consumer users because it would lead to a very bad practice.

stories/bar_chart.tsx

emmacunningham · 2019-07-02T09:06:08Z

src/lib/series/domains/y_domain.ts

-      } else if (customDomain && isUpperBound(customDomain)) {
-        if (computedDomainMin > customDomain.max) {
-          throw new Error(`custom yDomain for ${groupId} is invalid, computed min is greater than custom max`);
+      let domain: number[];


Should we use the Domain type here?

Yes and no. On the Y Axis we, currently can handle only numeric values, so the domain here is number[], or better: it should be [number, number]. What do you think?

ahhh yes, of course. Yes I agree [number, number]; maybe in the future we can consider how to tighten the Domain type so that we can between distinguish between types of domains so it is clearer.

src/lib/series/stacked_series_utils.ts

src/state/utils.ts

src/lib/series/series.ts

emmacunningham · 2019-07-02T09:29:07Z

src/lib/series/specs.ts

+  /**
+   * Stack each series in percentage for each point.
+   */
+  stackAsPercentage?: boolean;


It is possible to define a series with empty/undefined stackAccessors and stackAsPercentage=true. While functionally it ends up rendering ok, it seems we may want to consider how to constrain these "impossible states" in a way that makes it easy for the user to understand what options are available to them given certain other prop configurations.

I don't think this needs to be addressed in this PR because we also have a few other "impossible states" that can be configured currently, but wanted to start the discussion in case that is something we want to explore. This is a talk about how to achieve such a semantics in Elm, but certainly not unique to Elm and something that we could do using Typescript/React.

Sure, that should be addressed here: #85

nickofthyme

LGTM, verified locally.

src/lib/series/stacked_series_utils.ts

markov00 · 2019-07-04T14:06:05Z

@emmacunningham this is addressed on 3093ec9:

Also, should stackAsPercentage be unavailable as a prop on a line series? It seems to me that without the context that is provided by the area series, this prop with a line series could present very misleading data to the point that we shouldn't expose it as a possibility for consumer users because it would lead to a very bad practice.

emmacunningham

Changes LGTM!

Add the `stackAsPercentage` prop to allow stacking bars and areas computing the percentage of each data point on each bucket/x value. fix elastic#222

# [7.2.0](v7.1.0...v7.2.0) (2019-07-05) ### Bug Fixes * **ticks:** fill in additional ticks for histogram ([#251](#251)) ([af92736](af92736)) ### Features * **series:** stack series in percentage mode ([#250](#250)) ([1bfb430](1bfb430)), closes [#222](#222)

# [7.2.0](elastic/elastic-charts@v7.1.0...v7.2.0) (2019-07-05) ### Bug Fixes * **ticks:** fill in additional ticks for histogram ([opensearch-project#251](elastic/elastic-charts#251)) ([8045531](elastic/elastic-charts@8045531)) ### Features * **series:** stack series in percentage mode ([opensearch-project#250](elastic/elastic-charts#250)) ([892a826](elastic/elastic-charts@892a826)), closes [opensearch-project#222](elastic/elastic-charts#222)

feat(stack): stack series in percentage mode

d7ddda3

This commit will add a new props that allows stacking bars, areas and lines using the percentage mode. For each stacked x value, we scale their value to the percentage on that bucket. fix elastic#222

markov00 added :data Data/series/scales related issue enhancement New feature or request labels Jun 26, 2019

test: add tests for stackAsPercentage

230c7c6

markov00 requested review from emmacunningham and nickofthyme July 1, 2019 16:54

chore(storybook): add story for stacked as percentage

aa6f435

emmacunningham reviewed Jul 2, 2019

View reviewed changes

nickofthyme approved these changes Jul 3, 2019

View reviewed changes

markov00 added 4 commits July 3, 2019 15:44

fix(story): stackAccessor empty on clusterBar option on

e73de20

refactor(stacked_series): add StackedValues type

ef987e6

refactor(utils): rename updatesSeriesColor map name

7dd23ff

refactor(percentage_stack): check stackAsPercentage when split series

501289c

emmacunningham reviewed Jul 3, 2019

View reviewed changes

src/lib/series/stacked_series_utils.ts Show resolved Hide resolved

markov00 added 2 commits July 4, 2019 12:34

chore: add stacked area story

32bd14a

chore: allows stackAsPercentage only for areas and bars

3093ec9

markov00 requested a review from emmacunningham July 4, 2019 17:04

emmacunningham approved these changes Jul 5, 2019

View reviewed changes

markov00 merged this pull request into elastic:master Jul 5, 2019

markov00 mentioned this pull request Jul 5, 2019

fix(last_value): compute last value for non stacked series #261

Merged

2 tasks

flash1293 mentioned this pull request Jul 8, 2019

[Lens] Basic XY chart visualization extension elastic/kibana#36196

Closed

10 tasks

markov00 mentioned this pull request Jul 10, 2019

Upgrade @elastic/charts dependency to 7.2.1 elastic/kibana#40751

Merged

1 task

markov00 deleted the stack_percent branch November 25, 2020 11:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(series): stack series in percentage mode#250

feat(series): stack series in percentage mode#250
markov00 merged 9 commits intoelastic:masterfrom
markov00:stack_percent

markov00 commented Jun 26, 2019 •

edited

Loading

Uh oh!

codecov-io commented Jun 26, 2019 •

edited

Loading

Uh oh!

emmacunningham left a comment

Uh oh!

Uh oh!

emmacunningham Jul 2, 2019

Uh oh!

markov00 Jul 3, 2019

Uh oh!

emmacunningham Jul 3, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

emmacunningham Jul 2, 2019

Uh oh!

markov00 Jul 3, 2019

Uh oh!

nickofthyme left a comment

Uh oh!

Uh oh!

markov00 commented Jul 4, 2019

Uh oh!

emmacunningham left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

markov00 commented Jun 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

codecov-io commented Jun 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

emmacunningham left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

emmacunningham Jul 2, 2019

Choose a reason for hiding this comment

Uh oh!

markov00 Jul 3, 2019

Choose a reason for hiding this comment

Uh oh!

emmacunningham Jul 3, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

emmacunningham Jul 2, 2019

Choose a reason for hiding this comment

Uh oh!

markov00 Jul 3, 2019

Choose a reason for hiding this comment

Uh oh!

nickofthyme left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

markov00 commented Jul 4, 2019

Uh oh!

emmacunningham left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

markov00 commented Jun 26, 2019 •

edited

Loading

codecov-io commented Jun 26, 2019 •

edited

Loading