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.

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: graceful scale fallbacks and warnings #704

Merged
merged 5 commits into from
Jun 15, 2020

Conversation

nickofthyme
Copy link
Collaborator

@nickofthyme nickofthyme commented Jun 11, 2020

Summary

fix #678

Adds x scale fallback to ScaleType.Ordinal when there is a non-numeric value found in the dataset. This includes a warning for y values that are not numeric values. The current functionality is to error.

Each X value in a linear x scale needs be be a number. Using ordinal x scale as fallback.

Adds warning for y values that are not parable into a numeric value.

Found non-numeric y values in dataset for spec "areas" ("a", "c")

Example

String value 'a' among numeric values, converts scale from linear to ordinal

Image 2020-06-11 at 3 20 14 PM

Checklist

Delete any items that are not 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)
  • 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

@nickofthyme nickofthyme added the :xy Bar/Line/Area chart related label Jun 11, 2020
@codecov-commenter
Copy link

codecov-commenter commented Jun 11, 2020

Codecov Report

Merging #704 into master will increase coverage by 0.33%.
The diff coverage is 72.34%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #704      +/-   ##
==========================================
+ Coverage   74.80%   75.14%   +0.33%     
==========================================
  Files         264      280      +16     
  Lines        8514     8846     +332     
  Branches     1709     1760      +51     
==========================================
+ Hits         6369     6647     +278     
- Misses       2093     2141      +48     
- Partials       52       58       +6     
Impacted Files Coverage Δ
...ypes/partition_chart/renderer/canvas/partition.tsx 32.39% <ø> (ø)
src/utils/commons.ts 95.93% <ø> (-0.07%) ⬇️
src/utils/logger.ts 46.66% <46.66%> (ø)
src/state/chart_state.ts 87.09% <50.00%> (+0.21%) ⬆️
src/chart_types/xy_chart/domains/x_domain.ts 97.64% <71.42%> (-1.15%) ⬇️
src/chart_types/xy_chart/utils/series.ts 96.58% <88.88%> (-0.82%) ⬇️
src/chart_types/xy_chart/state/utils/utils.ts 92.95% <100.00%> (+0.50%) ⬆️
src/mocks/specs/specs.ts 74.71% <0.00%> (ø)
src/mocks/theme.ts 100.00% <0.00%> (ø)
src/mocks/specs/index.ts 100.00% <0.00%> (ø)
... and 13 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 a5f48f7...f587d6c. Read the comment docs.

@nickofthyme
Copy link
Collaborator Author

nickofthyme commented Jun 15, 2020

@markov00 I'm not sure about needed to have them separate. I think eventually the chart should error in dev to alert the developer of bad config then in prod only console.error severe issues.

I'll make the change for now and see if my opinion changes too.

see 5c42e8b

Copy link
Member

@markov00 markov00 left a comment

Choose a reason for hiding this comment

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

changes LGTM

@nickofthyme nickofthyme merged commit ed49bbb into elastic:master Jun 15, 2020
@nickofthyme nickofthyme deleted the fix/x-domain-fallback branch June 15, 2020 15:50
markov00 pushed a commit that referenced this pull request Jun 16, 2020
## [19.5.1](v19.5.0...v19.5.1) (2020-06-16)

### Bug Fixes

* graceful scale fallbacks and warnings ([#704](#704)) ([ed49bbb](ed49bbb)), closes [#678](#678)
* **axis:** rotated label positioning ([#709](#709)) ([2e26430](2e26430)), closes [#673](#673)
* **tooltip:** popper scroll issue ([#712](#712)) ([0c97c67](0c97c67))
@markov00
Copy link
Member

🎉 This PR is included in version 19.5.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@markov00 markov00 added the released Issue released publicly label Jun 16, 2020
AMoo-Miki pushed a commit to AMoo-Miki/OpenSearch-Dashboards that referenced this pull request Feb 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released Issue released publicly :xy Bar/Line/Area chart related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

XDomain should fallback to ordinal type
3 participants