Skip to content

Commit

Permalink
fix: correct the formula for interpolating between bin start and end …
Browse files Browse the repository at this point in the history
…(interpolatedSignalRef) (#9111)

Co-authored-by: GitHub Actions Bot <vega-actions-bot@users.noreply.github.com>
  • Loading branch information
kanitw and GitHub Actions Bot committed Sep 29, 2023
1 parent 31e1add commit d4b27bb
Show file tree
Hide file tree
Showing 10 changed files with 31 additions and 29 deletions.
Binary file modified examples/compiled/bar_month_band.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion examples/compiled/bar_month_band.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 2 additions & 2 deletions examples/compiled/bar_month_band.vg.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,13 @@
"signal": "\"date (month): \" + (timeFormat(datum[\"month_date\"], timeUnitSpecifier([\"month\"], {\"year-month\":\"%b %Y \",\"year-month-date\":\"%b %d, %Y \"}))) + \"; Mean of precipitation: \" + (format(datum[\"mean_precipitation\"], \"\"))"
},
"x2": {
"signal": "scale(\"x\", 0.15000000000000002 * datum[\"month_date\"] + 0.85 * datum[\"month_date_end\"])",
"signal": "scale(\"x\", 0.85 * datum[\"month_date\"] + 0.15000000000000002 * datum[\"month_date_end\"])",
"offset": {
"signal": "0.5 + (abs(scale(\"x\", datum[\"month_date_end\"]) - scale(\"x\", datum[\"month_date\"])) < 0.25 ? -0.5 * (0.25 - (abs(scale(\"x\", datum[\"month_date_end\"]) - scale(\"x\", datum[\"month_date\"])))) : 0.5)"
}
},
"x": {
"signal": "scale(\"x\", 0.85 * datum[\"month_date\"] + 0.15000000000000002 * datum[\"month_date_end\"])",
"signal": "scale(\"x\", 0.15000000000000002 * datum[\"month_date\"] + 0.85 * datum[\"month_date_end\"])",
"offset": {
"signal": "0.5 + (abs(scale(\"x\", datum[\"month_date_end\"]) - scale(\"x\", datum[\"month_date\"])) < 0.25 ? 0.5 * (0.25 - (abs(scale(\"x\", datum[\"month_date_end\"]) - scale(\"x\", datum[\"month_date\"])))) : -0.5)"
}
Expand Down
Binary file modified examples/compiled/bar_month_band_config.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion examples/compiled/bar_month_band_config.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 2 additions & 2 deletions examples/compiled/bar_month_band_config.vg.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,13 @@
"signal": "\"date (month): \" + (timeFormat(datum[\"month_date\"], timeUnitSpecifier([\"month\"], {\"year-month\":\"%b %Y \",\"year-month-date\":\"%b %d, %Y \"}))) + \"; Mean of precipitation: \" + (format(datum[\"mean_precipitation\"], \"\"))"
},
"x2": {
"signal": "scale(\"x\", 0.15000000000000002 * datum[\"month_date\"] + 0.85 * datum[\"month_date_end\"])",
"signal": "scale(\"x\", 0.85 * datum[\"month_date\"] + 0.15000000000000002 * datum[\"month_date_end\"])",
"offset": {
"signal": "0.5 + (abs(scale(\"x\", datum[\"month_date_end\"]) - scale(\"x\", datum[\"month_date\"])) < 0.25 ? -0.5 * (0.25 - (abs(scale(\"x\", datum[\"month_date_end\"]) - scale(\"x\", datum[\"month_date\"])))) : 0.5)"
}
},
"x": {
"signal": "scale(\"x\", 0.85 * datum[\"month_date\"] + 0.15000000000000002 * datum[\"month_date_end\"])",
"signal": "scale(\"x\", 0.15000000000000002 * datum[\"month_date\"] + 0.85 * datum[\"month_date_end\"])",
"offset": {
"signal": "0.5 + (abs(scale(\"x\", datum[\"month_date_end\"]) - scale(\"x\", datum[\"month_date\"])) < 0.25 ? 0.5 * (0.25 - (abs(scale(\"x\", datum[\"month_date_end\"]) - scale(\"x\", datum[\"month_date\"])))) : -0.5)"
}
Expand Down
24 changes: 12 additions & 12 deletions src/compile/data/timeunit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,12 @@ function offsetAs(field: FieldName) {

export class TimeUnitNode extends DataFlowNode {
public clone() {
return new TimeUnitNode(null, duplicate(this.formula));
return new TimeUnitNode(null, duplicate(this.timeUnits));
}

constructor(
parent: DataFlowNode,
private formula: Dict<TimeUnitComponent>
private timeUnits: Dict<TimeUnitComponent>
) {
super(parent);
}
Expand Down Expand Up @@ -101,13 +101,13 @@ export class TimeUnitNode extends DataFlowNode {
* and removing `other`.
*/
public merge(other: TimeUnitNode) {
this.formula = {...this.formula};
this.timeUnits = {...this.timeUnits};

// if the same hash happen twice, merge
for (const key in other.formula) {
if (!this.formula[key]) {
for (const key in other.timeUnits) {
if (!this.timeUnits[key]) {
// copy if it's not a duplicate
this.formula[key] = other.formula[key];
this.timeUnits[key] = other.timeUnits[key];
}
}

Expand All @@ -125,7 +125,7 @@ export class TimeUnitNode extends DataFlowNode {
public removeFormulas(fields: Set<string>) {
const newFormula = {};

for (const [key, timeUnitComponent] of entries(this.formula)) {
for (const [key, timeUnitComponent] of entries(this.timeUnits)) {
const fieldAs = isTimeUnitTransformComponent(timeUnitComponent)
? timeUnitComponent.as
: `${timeUnitComponent.field}_end`;
Expand All @@ -134,29 +134,29 @@ export class TimeUnitNode extends DataFlowNode {
}
}

this.formula = newFormula;
this.timeUnits = newFormula;
}

public producedFields() {
return new Set(
vals(this.formula).map(f => {
vals(this.timeUnits).map(f => {
return isTimeUnitTransformComponent(f) ? f.as : offsetAs(f.field);
})
);
}

public dependentFields() {
return new Set(vals(this.formula).map(f => f.field));
return new Set(vals(this.timeUnits).map(f => f.field));
}

public hash() {
return `TimeUnit ${hash(this.formula)}`;
return `TimeUnit ${hash(this.timeUnits)}`;
}

public assemble() {
const transforms: (VgTimeUnitTransform | VgFormulaTransform)[] = [];

for (const f of vals(this.formula)) {
for (const f of vals(this.timeUnits)) {
if (isTimeUnitTransformComponent(f)) {
const {field, as, timeUnit} = f;
const {unit, utc, ...params} = normalizeTimeUnit(timeUnit);
Expand Down
18 changes: 10 additions & 8 deletions src/compile/mark/encode/position-rect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ function getBinSpacing(
channel: PositionChannel | PolarPositionChannel,
spacing: number,
reverse: boolean | SignalRef,
translate: number | SignalRef,
axisTranslate: number | SignalRef,
offset: number | VgValueRef,
minBandSize: number | SignalRef,
bandSizeExpr: string
Expand All @@ -255,10 +255,10 @@ function getBinSpacing(

const spacingOffset = isEnd ? -spacing / 2 : spacing / 2;

if (isSignalRef(reverse) || isSignalRef(offset) || isSignalRef(translate) || minBandSize) {
if (isSignalRef(reverse) || isSignalRef(offset) || isSignalRef(axisTranslate) || minBandSize) {
const reverseExpr = signalOrStringValue(reverse);
const offsetExpr = signalOrStringValue(offset);
const translateExpr = signalOrStringValue(translate);
const axisTranslateExpr = signalOrStringValue(axisTranslate);
const minBandSizeExpr = signalOrStringValue(minBandSize);

const sign = isEnd ? '' : '-';
Expand All @@ -267,7 +267,7 @@ function getBinSpacing(
? `(${bandSizeExpr} < ${minBandSizeExpr} ? ${sign}0.5 * (${minBandSizeExpr} - (${bandSizeExpr})) : ${spacingOffset})`
: spacingOffset;

const t = translateExpr ? `${translateExpr} + ` : '';
const t = axisTranslateExpr ? `${axisTranslateExpr} + ` : '';
const r = reverseExpr ? `(${reverseExpr} ? -1 : 1) * ` : '';
const o = offsetExpr ? `(${offsetExpr} + ${spacingAndSizeOffset})` : spacingAndSizeOffset;

Expand All @@ -276,7 +276,7 @@ function getBinSpacing(
};
} else {
offset = offset || 0;
return translate + (reverse ? -offset - spacingOffset : +offset + spacingOffset);
return axisTranslate + (reverse ? -offset - spacingOffset : +offset + spacingOffset);
}
}

Expand Down Expand Up @@ -325,7 +325,7 @@ function rectBinPosition({
bandSizeExpr
);

const bandPosition = isSignalRef(bandSize)
const bandPositionForBandSize = isSignalRef(bandSize)
? {signal: `(1-${bandSize.signal})/2`}
: isRelativeBandSize(bandSize)
? (1 - bandSize.band) / 2
Expand All @@ -336,13 +336,15 @@ function rectBinPosition({
[vgChannel2]: rectBinRef({
fieldDef,
scaleName,
bandPosition,
bandPosition: bandPositionForBandSize,
offset: binSpacingOffset2
}),
[vgChannel]: rectBinRef({
fieldDef,
scaleName,
bandPosition: isSignalRef(bandPosition) ? {signal: `1-${bandPosition.signal}`} : 1 - bandPosition,
bandPosition: isSignalRef(bandPositionForBandSize)
? {signal: `1-${bandPositionForBandSize.signal}`}
: 1 - bandPositionForBandSize,
offset: binSpacingOffset
})
};
Expand Down
4 changes: 2 additions & 2 deletions src/compile/mark/encode/valueref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,8 @@ export function interpolatedSignalRef({
ref.field = field;
} else {
const datum = isSignalRef(bandPosition)
? `${bandPosition.signal} * ${start} + (1-${bandPosition.signal}) * ${end}`
: `${bandPosition} * ${start} + ${1 - bandPosition} * ${end}`;
? `(1-${bandPosition.signal}) * ${start} + ${bandPosition.signal} * ${end}`
: `${1 - bandPosition} * ${start} + ${bandPosition} * ${end}`;
ref.signal = `scale("${scaleName}", ${datum})`;
}

Expand Down
2 changes: 1 addition & 1 deletion test/compile/mark/point.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ describe('Mark: Point', () => {
const props = point.encodeEntry(model);

expect(props.x).toEqual({
signal: 'scale("x", 0.6 * datum["bin_maxbins_10_a"] + 0.4 * datum["bin_maxbins_10_a_end"])'
signal: 'scale("x", 0.4 * datum["bin_maxbins_10_a"] + 0.6 * datum["bin_maxbins_10_a_end"])'
});
});
it('interpolates x timeUnit with timeUnitBand = 0.5', () => {
Expand Down

0 comments on commit d4b27bb

Please sign in to comment.