Skip to content

Commit

Permalink
fix: default nice to false if domainMin/domainMax are set (#7802)
Browse files Browse the repository at this point in the history
Co-authored-by: Mac Lockard <maclockard@gmail.com>
  • Loading branch information
domoritz and maclockard committed Nov 14, 2021
1 parent 5e416f4 commit 946a559
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 7 deletions.
2 changes: 1 addition & 1 deletion build/vega-lite-schema.json
Expand Up @@ -21198,7 +21198,7 @@
"$ref": "#/definitions/ExprRef"
}
],
"description": "Extending the domain so that it starts and ends on nice round values. This method typically modifies the scale’s domain, and may only extend the bounds to the nearest round value. Nicing is useful if the domain is computed from data and may be irregular. For example, for a domain of _[0.201479…, 0.996679…]_, a nice domain might be _[0.2, 1.0]_.\n\nFor quantitative scales such as linear, `nice` can be either a boolean flag or a number. If `nice` is a number, it will represent a desired tick count. This allows greater control over the step size used to extend the bounds, guaranteeing that the returned ticks will exactly cover the domain.\n\nFor temporal fields with time and utc scales, the `nice` value can be a string indicating the desired time interval. Legal values are `\"millisecond\"`, `\"second\"`, `\"minute\"`, `\"hour\"`, `\"day\"`, `\"week\"`, `\"month\"`, and `\"year\"`. Alternatively, `time` and `utc` scales can accept an object-valued interval specifier of the form `{\"interval\": \"month\", \"step\": 3}`, which includes a desired number of interval steps. Here, the domain would snap to quarter (Jan, Apr, Jul, Oct) boundaries.\n\n__Default value:__ `true` for unbinned _quantitative_ fields; `false` otherwise."
"description": "Extending the domain so that it starts and ends on nice round values. This method typically modifies the scale’s domain, and may only extend the bounds to the nearest round value. Nicing is useful if the domain is computed from data and may be irregular. For example, for a domain of _[0.201479…, 0.996679…]_, a nice domain might be _[0.2, 1.0]_.\n\nFor quantitative scales such as linear, `nice` can be either a boolean flag or a number. If `nice` is a number, it will represent a desired tick count. This allows greater control over the step size used to extend the bounds, guaranteeing that the returned ticks will exactly cover the domain.\n\nFor temporal fields with time and utc scales, the `nice` value can be a string indicating the desired time interval. Legal values are `\"millisecond\"`, `\"second\"`, `\"minute\"`, `\"hour\"`, `\"day\"`, `\"week\"`, `\"month\"`, and `\"year\"`. Alternatively, `time` and `utc` scales can accept an object-valued interval specifier of the form `{\"interval\": \"month\", \"step\": 3}`, which includes a desired number of interval steps. Here, the domain would snap to quarter (Jan, Apr, Jul, Oct) boundaries.\n\n__Default value:__ `true` for unbinned _quantitative_ fields without explicit domain bounds; `false` otherwise."
},
"padding": {
"anyOf": [
Expand Down
11 changes: 10 additions & 1 deletion src/compile/scale/properties.ts
Expand Up @@ -116,6 +116,8 @@ function parseUnitScaleProperty(model: UnitModel, property: Exclude<keyof (Scale
scalePadding,
scalePaddingInner,
domain: specifiedScale.domain,
domainMin: specifiedScale.domainMin,
domainMax: specifiedScale.domainMax,
markDef,
config,
hasNestedOffsetScale: channelHasNestedOffsetScale(encoding, channel)
Expand All @@ -138,6 +140,8 @@ export interface ScaleRuleParams {
scalePadding: number | SignalRef;
scalePaddingInner: number | SignalRef;
domain: Domain;
domainMin: Scale['domainMin'];
domainMax: Scale['domainMax'];
markDef: MarkDef<Mark, SignalRef>;
config: Config<SignalRef>;
}
Expand All @@ -149,7 +153,8 @@ export const scaleRules: {

interpolate: ({channel, fieldOrDatumDef}) => interpolate(channel, fieldOrDatumDef.type),

nice: ({scaleType, channel, domain, fieldOrDatumDef}) => nice(scaleType, channel, domain, fieldOrDatumDef),
nice: ({scaleType, channel, domain, domainMin, domainMax, fieldOrDatumDef}) =>
nice(scaleType, channel, domain, domainMin, domainMax, fieldOrDatumDef),

padding: ({channel, scaleType, fieldOrDatumDef, markDef, config}) =>
padding(channel, scaleType, config.scale, fieldOrDatumDef, markDef, config.bar),
Expand Down Expand Up @@ -246,11 +251,15 @@ export function nice(
scaleType: ScaleType,
channel: ScaleChannel,
specifiedDomain: Domain,
domainMin: Scale['domainMin'],
domainMax: Scale['domainMax'],
fieldOrDatumDef: TypedFieldDef<string> | ScaleDatumDef
): boolean | TimeInterval {
if (
getFieldDef(fieldOrDatumDef)?.bin ||
isArray(specifiedDomain) ||
domainMax != null ||
domainMin != null ||
util.contains([ScaleType.TIME, ScaleType.UTC], scaleType)
) {
return undefined;
Expand Down
2 changes: 1 addition & 1 deletion src/scale.ts
Expand Up @@ -673,7 +673,7 @@ export interface Scale<ES extends ExprRef | SignalRef = ExprRef | SignalRef> {
*
* For temporal fields with time and utc scales, the `nice` value can be a string indicating the desired time interval. Legal values are `"millisecond"`, `"second"`, `"minute"`, `"hour"`, `"day"`, `"week"`, `"month"`, and `"year"`. Alternatively, `time` and `utc` scales can accept an object-valued interval specifier of the form `{"interval": "month", "step": 3}`, which includes a desired number of interval steps. Here, the domain would snap to quarter (Jan, Apr, Jul, Oct) boundaries.
*
* __Default value:__ `true` for unbinned _quantitative_ fields; `false` otherwise.
* __Default value:__ `true` for unbinned _quantitative_ fields without explicit domain bounds; `false` otherwise.
*
*/
nice?: boolean | number | TimeInterval | TimeIntervalStep | ES;
Expand Down
22 changes: 18 additions & 4 deletions test/compile/scale/properties.test.ts
Expand Up @@ -7,25 +7,39 @@ describe('compile/scale', () => {
describe('nice', () => {
it('should return nice for x and y', () => {
for (const c of ['x', 'y'] as const) {
expect(rules.nice('linear', c, undefined, {type: 'quantitative'})).toBe(true);
expect(rules.nice('linear', c, undefined, undefined, undefined, {type: 'quantitative'})).toBe(true);
}
});

it('should not return nice for binned x and y', () => {
for (const c of ['x', 'y'] as const) {
expect(rules.nice('linear', c, undefined, {type: 'quantitative', field: 'a', bin: true})).toBeUndefined();
expect(
rules.nice('linear', c, undefined, undefined, undefined, {type: 'quantitative', field: 'a', bin: true})
).toBeUndefined();
}
});

it('should not return nice for temporal x and y', () => {
for (const c of ['x', 'y'] as const) {
expect(rules.nice('time', c, undefined, {type: 'temporal'})).toBeUndefined();
expect(rules.nice('time', c, undefined, undefined, undefined, {type: 'temporal'})).toBeUndefined();
}
});

it('should not return nice when domain is set explicitly', () => {
for (const c of ['x', 'y'] as const) {
expect(rules.nice('time', c, [0, 42], {type: 'quantitative'})).toBeUndefined();
expect(rules.nice('time', c, [0, 42], undefined, undefined, {type: 'quantitative'})).toBeUndefined();
}
});

it('should not return nice when domainMin is set explicitly', () => {
for (const c of ['x', 'y'] as const) {
expect(rules.nice('time', c, undefined, 0, undefined, {type: 'quantitative'})).toBeUndefined();
}
});

it('should not return nice when domainMax is set explicitly', () => {
for (const c of ['x', 'y'] as const) {
expect(rules.nice('time', c, [0, 42], undefined, 0, {type: 'quantitative'})).toBeUndefined();
}
});
});
Expand Down

0 comments on commit 946a559

Please sign in to comment.