Skip to content

Fix bug when calculating if steps fit into scale as a whole number th… - #3685

Merged
etimberg merged 1 commit into
chartjs:masterfrom
Tarqwyn:floating_point_bug
Dec 14, 2016
Merged

etimberg merged 1 commit into
chartjs:masterfrom
Tarqwyn:floating_point_bug

Conversation

@Tarqwyn

@Tarqwyn Tarqwyn commented Dec 6, 2016 •

Copy link
Copy Markdown
Contributor

This PR fixes an error where if generationOptions.max or generationOptions.min is calculated to an almost whole number (ie 9.999) then we display the number as 10 on the scale, but calculating an evenly spaced scale is not possible (eg stepSize is 2) as the check fails.

By applying an almost whole check we are able to mitigate for FP arithmetic inconsistencies that would cause this behaviour

Please consider the following before submitting a pull request:

Guidelines for contributing: https://github.com/chartjs/Chart.js/blob/master/CONTRIBUTING.md

Example of changes on an interactive website such as the following:

@Tarqwyn
Tarqwyn force-pushed the floating_point_bug branch 2 times, most recently from 8249534 to 8f73bd2 Compare December 6, 2016 15:32
…en smal floating point errors make the consition pass false
@Tarqwyn
Tarqwyn force-pushed the floating_point_bug branch from 8f73bd2 to 19d26ac Compare December 7, 2016 09:29

@etimberg etimberg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this is fine. CC @chartjs/maintainers

@etimberg
etimberg merged commit 34d26ea into chartjs:master Dec 14, 2016
@simonbrunel simonbrunel added this to the Version 2.5 milestone Jan 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants