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

Jump to bottom

Add ticks.integerSteps option to linear scale. #4841

Merged
merged 4 commits into from Apr 1, 2018
Merged

Add ticks.integerSteps option to linear scale. #4841

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
14efb12
Add `ticks.integerSteps` option to linear scale. When true, and
etimberg Oct 15, 2017
b934ca8
Implement precision config option
etimberg Jan 14, 2018
7126966
fix linting
etimberg Jan 14, 2018
11fcc1d
Update variable names
etimberg Feb 16, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/axes/cartesian/linear.md
View file
Open in desktop
Expand Up @@ -12,6 +12,7 @@ The following options are provided by the linear scale. They are all located in
| `min` | `Number` | | User defined minimum number for the scale, overrides minimum value from data. [more...](#axis-range-settings)
| `max` | `Number` | | User defined maximum number for the scale, overrides maximum value from data. [more...](#axis-range-settings)
| `maxTicksLimit` | `Number` | `11` | Maximum number of ticks and gridlines to show.
| `precision` | `Number` | | if defined and `stepSize` is not specified, the step size will be rounded to this many decimal places.
| `stepSize` | `Number` | | User defined fixed step size for the scale. [more...](#step-size)
| `suggestedMax` | `Number` | | Adjustment used when calculating the maximum data value. [more...](#axis-range-settings)
| `suggestedMin` | `Number` | | Adjustment used when calculating the minimum data value. [more...](#axis-range-settings)
Expand Down
1 change: 1 addition & 0 deletions docs/axes/radial/linear.md
View file
Open in desktop
Expand Up @@ -27,6 +27,7 @@ The following options are provided by the linear scale. They are all located in
| `min` | `Number` | | User defined minimum number for the scale, overrides minimum value from data. [more...](#axis-range-settings)
| `max` | `Number` | | User defined maximum number for the scale, overrides maximum value from data. [more...](#axis-range-settings)
| `maxTicksLimit` | `Number` | `11` | Maximum number of ticks and gridlines to show.
| `precision` | `Number` | | if defined and `stepSize` is not specified, the step size will be rounded to this many decimal places.
| `stepSize` | `Number` | | User defined fixed step size for the scale. [more...](#step-size)
| `suggestedMax` | `Number` | | Adjustment used when calculating the maximum data value. [more...](#axis-range-settings)
| `suggestedMin` | `Number` | | Adjustment used when calculating the minimum data value. [more...](#axis-range-settings)
Expand Down
9 changes: 9 additions & 0 deletions src/scales/scale.linearbase.js
View file
Open in desktop
Expand Up @@ -15,11 +15,19 @@ function generateTicks(generationOptions, dataRange) {
// for details.

var spacing;

if (generationOptions.stepSize && generationOptions.stepSize > 0) {
spacing = generationOptions.stepSize;
} else {
var niceRange = helpers.niceNum(dataRange.max - dataRange.min, false);
spacing = helpers.niceNum(niceRange / (generationOptions.maxTicks - 1), true);

var spacingPrecision = generationOptions.precision;
Copy link
Contributor

Choose a reason for hiding this comment

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

@etimberg I think it would be better rename the previous precision variable to factor and reuse it, instead of introducing the variable spacePrecision. This to improve readability and as it serves the same purpose later in the function.

function generateTicks(generationOptions, dataRange) {
	var ticks = [];
	// To get a "nice" value for the tick spacing, we will use the appropriately named
	// "nice number" algorithm. See http://stackoverflow.com/questions/8506881/nice-label-algorithm-for-charts-with-minimum-ticks
	// for details.

	var factor = 1;
	var spacing;

	if (generationOptions.stepSize && generationOptions.stepSize > 0) {
		spacing = generationOptions.stepSize;
	} else {
		var niceRange = helpers.niceNum(dataRange.max - dataRange.min, false);
		spacing = helpers.niceNum(niceRange / (generationOptions.maxTicks - 1), true);
		if (generationOptions.precision !== undefined) {
			factor = Math.pow(10, generationOptions.precision);
			spacing = Math.ceil(spacing * factor) / factor;
		}
	}
	var niceMin = Math.floor(dataRange.min / spacing) * spacing;
	var niceMax = Math.ceil(dataRange.max / spacing) * spacing;

	// If min, max and stepSize is set and they make an evenly spaced scale use it.
	if (generationOptions.min && generationOptions.max && generationOptions.stepSize) {
		// If very close to our whole number, use it.
		if (helpers.almostWhole((generationOptions.max - generationOptions.min) / generationOptions.stepSize, spacing / 1000)) {
			niceMin = generationOptions.min;
			niceMax = generationOptions.max;
		}
	}

	var numSpaces = (niceMax - niceMin) / spacing;
	// If very close to our rounded value, use it.
	if (helpers.almostEquals(numSpaces, Math.round(numSpaces), spacing / 1000)) {
		numSpaces = Math.round(numSpaces);
	} else {
		numSpaces = Math.ceil(numSpaces);
	}

	if (spacing < 1) {
		factor = Math.pow(10, spacing.toString().length - 2);
		niceMin = Math.round(niceMin * factor) / factor;
		niceMax = Math.round(niceMax * factor) / factor;
	}
	ticks.push(generationOptions.min !== undefined ? generationOptions.min : niceMin);
	for (var j = 1; j < numSpaces; ++j) {
		ticks.push(Math.round((niceMin + j * spacing) * factor) / factor);
	}
	ticks.push(generationOptions.max !== undefined ? generationOptions.max : niceMax);

	return ticks;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I think precision is more understandable to end users. E.g. if you want to round to the nearest .01 it's pretty easy to see that this is two decimal places, but I'm not sure we would want to make the user calculate that they should pass in 100

Copy link
Contributor

Choose a reason for hiding this comment

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

@benmccann I am not sure I understand your comment. My proposal wasn't to change the precision-option given by the user, but to use the variable precision inside the function, instead of introducing an extra variable spacingPrecision.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or more precisely renaming the previous use of precision to factor and use this for both tick spacing as tick rounding, as they both serve the same purpose.

Copy link
Member

Choose a reason for hiding this comment

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

I agree with @jcopperfield, it's more readable renaming internal variables precision -> factor and spacingPrecision -> precision since these have different units / meanings.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, will make this change

if (spacingPrecision !== undefined) {
// If the user specified a precision, round to that number of decimal places
var factor = Math.pow(10, spacingPrecision);
spacing = Math.ceil(spacing * factor) / factor;
}
}
var niceMin = Math.floor(dataRange.min / spacing) * spacing;
var niceMax = Math.ceil(dataRange.max / spacing) * spacing;
Expand Down Expand Up @@ -154,6 +162,7 @@ module.exports = function(Chart) {
maxTicks: maxTicks,
min: tickOpts.min,
max: tickOpts.max,
precision: tickOpts.precision,
stepSize: helpers.valueOrDefault(tickOpts.fixedStepSize, tickOpts.stepSize)
};
var ticks = me.ticks = generateTicks(numericGeneratorOptions, me);
Expand Down
60 changes: 60 additions & 0 deletions test/specs/scale.linear.tests.js
View file
Open in desktop
Expand Up @@ -548,6 +548,66 @@ describe('Linear Scale', function() {
expect(chart.scales.yScale0.ticks).toEqual(['11', '9', '7', '5', '3', '1']);
});

describe('precision', function() {
it('Should create integer steps if precision is 0', function() {
var chart = window.acquireChart({
type: 'bar',
data: {
datasets: [{
yAxisID: 'yScale0',
data: [0, 1, 2, 1, 0, 1]
}],
labels: ['a', 'b', 'c', 'd', 'e', 'f']
},
options: {
scales: {
yAxes: [{
id: 'yScale0',
type: 'linear',
ticks: {
precision: 0
}
}]
}
}
});

expect(chart.scales.yScale0).not.toEqual(undefined); // must construct
expect(chart.scales.yScale0.min).toBe(0);
expect(chart.scales.yScale0.max).toBe(2);
expect(chart.scales.yScale0.ticks).toEqual(['2', '1', '0']);
});

it('Should round the step size to the given number of decimal places', function() {
var chart = window.acquireChart({
type: 'bar',
data: {
datasets: [{
yAxisID: 'yScale0',
data: [0, 0.001, 0.002, 0.003, 0, 0.001]
}],
labels: ['a', 'b', 'c', 'd', 'e', 'f']
},
options: {
scales: {
yAxes: [{
id: 'yScale0',
type: 'linear',
ticks: {
precision: 2
}
}]
}
}
});

expect(chart.scales.yScale0).not.toEqual(undefined); // must construct
expect(chart.scales.yScale0.min).toBe(0);
expect(chart.scales.yScale0.max).toBe(0.01);
expect(chart.scales.yScale0.ticks).toEqual(['0.01', '0']);
});
});


it('should forcibly include 0 in the range if the beginAtZero option is used', function() {
var chart = window.acquireChart({
Expand Down