From e0b199463cdc6a0716ad61cc971080007cfe0c70 Mon Sep 17 00:00:00 2001 From: Shaheen Sharifian Date: Wed, 1 Aug 2018 10:25:21 -0700 Subject: [PATCH] Fix bar to point bug (#445) --- .vscode/launch.json | 23 ++++++----------------- package.json | 1 + src/constraint/spec.ts | 6 +++--- src/model.ts | 11 ++++++++--- test/recommend.test.ts | 42 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 60 insertions(+), 23 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 3d16a8aa..1f5f7d31 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -1,12 +1,13 @@ { "version": "0.2.0", - "configurations": [{ - "name": "Launch", + "configurations": [ + { + "name": "Tests", "type": "node", "request": "launch", - "program": "${workspaceRoot}/compassql.js", + "program": "${workspaceRoot}/node_modules/jest-cli/bin/jest.js", "stopOnEntry": false, - "args": [], + "args": ["--runInBand"], "cwd": "${workspaceRoot}", "preLaunchTask": null, "runtimeExecutable": null, @@ -17,20 +18,8 @@ "NODE_ENV": "development" }, "externalConsole": false, - "sourceMaps": true, + "sourceMaps": false, "outDir": null - }, - { - "name": "Attach", - "type": "node", - "request": "attach", - "port": 5858, - "address": "localhost", - "restart": false, - "sourceMaps": true, - "outFiles": ["${workspaceRoot}/build/**/*.js"], - "localRoot": "${workspaceRoot}", - "remoteRoot": null } ] } \ No newline at end of file diff --git a/package.json b/package.json index 2e6a793f..b03c22ec 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,7 @@ "lint": "tslint -c tslint.json src/**/*.ts test/**/*.ts", "schema": "npm run prebuild && typescript-json-schema --required true src/query.ts Query > build/compassql-schema.json", "test": "jest --maxWorkers=4 && npm run lint", + "test:inspect": "node --inspect-brk ./node_modules/.bin/jest --runInBand", "check:examples": "./scripts/check-examples.sh", "watch:build": "npm run build && concurrently --kill-others -n Typescript,Rollup 'tsc -w' 'rollup -c -w'", "watch:test": "jest --watch" diff --git a/src/constraint/spec.ts b/src/constraint/spec.ts index e93a937b..4e4983ad 100644 --- a/src/constraint/spec.ts +++ b/src/constraint/spec.ts @@ -141,7 +141,7 @@ export const SPEC_CONSTRAINTS: SpecConstraintModel[] = [ // Auto count should only be applied if all fields are nominal, ordinal, temporal with timeUnit, binned quantitative, or autoCount return every(specM.getEncodings(), (encQ: EncodingQuery) => { if (isValueQuery(encQ)) { - return false; + return true; } if (isAutoCountQuery(encQ)) { @@ -333,7 +333,7 @@ export const SPEC_CONSTRAINTS: SpecConstraintModel[] = [ satisfy: (specM: SpecQueryModel, _: Schema, opt: QueryConfig) => { const mark = specM.getMark(); if (contains([Mark.TICK, Mark.BAR], mark)) { - if (specM.channelUsed(Channel.SIZE)) { + if (specM.channelEncodingField(Channel.SIZE)) { if (opt.constraintManuallySpecifiedValue) { // If size is used and we constraintManuallySpecifiedValue, // then the spec violates this constraint. @@ -641,7 +641,7 @@ export const SPEC_CONSTRAINTS: SpecConstraintModel[] = [ case Mark.BAR: case Mark.TICK: // Bar and tick should not use size. - if (specM.channelUsed(Channel.SIZE)) { + if (specM.channelEncodingField(Channel.SIZE)) { return false; } else { // Tick and Bar should have one and only one measure diff --git a/src/model.ts b/src/model.ts index 4d0ab06c..3cacc5de 100644 --- a/src/model.ts +++ b/src/model.ts @@ -69,7 +69,7 @@ export class SpecQueryModel { // For each property of the encodingQuery, enumerate ENCODING_TOPLEVEL_PROPS.forEach((prop) => { - if(isWildcard(encQ[prop])) { + if (isWildcard(encQ[prop])) { // Assign default wildcard name and enum values. const defaultWildcardName = getDefaultName(prop) + index; const defaultEnumValues = getDefaultEnumValues(prop, schema, opt); @@ -230,6 +230,11 @@ export class SpecQueryModel { return this._channelFieldCount[channel] > 0; } + public channelEncodingField(channel: Channel) { + const encodingQuery = this.getEncodingQueryByChannel(channel); + return isFieldQuery(encodingQuery); + } + public getEncodings(): EncodingQuery[] { // do not include encoding that has autoCount = false because it is not a part of the output spec. return this._spec.encodings.filter(encQ => !isDisabledAutoCountQuery(encQ)); @@ -319,7 +324,7 @@ export class SpecQueryModel { if (this._spec.padding) { spec.padding = this._spec.padding; } - if(this._spec.title) { + if (this._spec.title) { spec.title = this._spec.title; } @@ -327,7 +332,7 @@ export class SpecQueryModel { return null; } if (this._spec.config || this._opt.defaultSpecConfig) - spec.config = extend({}, this._opt.defaultSpecConfig, this._spec.config); + spec.config = extend({}, this._opt.defaultSpecConfig, this._spec.config); return spec; } diff --git a/test/recommend.test.ts b/test/recommend.test.ts index 68dcd772..84332f9a 100644 --- a/test/recommend.test.ts +++ b/test/recommend.test.ts @@ -52,6 +52,48 @@ describe('recommend()', () => { assert.equal(getTopResultTreeItem(group.result).getMark(), 'line'); }); + it('recommends bar chart given 1 nominal field and specifying value for size channel', () => { + const group = recommend({ + "spec": { + "data": {"url": "data/cars.json"}, + "mark": "?", + "encodings": [ + {"channel": "?","field": "Origin","type": "nominal"}, + {"channel": "size", value: 52} + ] + }, + "nest": [ + { + "groupBy": ["field","aggregate","bin","timeUnit","stack"], + "orderGroupBy": "aggregationQuality" + }, + { + "groupBy": [ + { + "property": "channel", + "replace": { + "x": "xy", + "y": "xy", + "color": "style", + "size": "style", + "shape": "style", + "opacity": "style", + "row": "facet", + "column": "facet" + } + } + ], + "orderGroupBy": "effectiveness" + }, + {"groupBy": ["channel"],"orderGroupBy": "effectiveness"} + ], + "orderBy": "effectiveness", + "config": {"autoAddCount": true} + }, schema); + + assert.equal(getTopResultTreeItem(group.result).getMark(), 'bar'); + }); + it('recommends bar for a histogram of a temporal field', () => { const group = recommend({ "spec": {