Repository: vega/vega-lite
Pull request #612 (2015-10-07 05:27:14)

Vega 2 Sort 
(part of #459)
- remove `sort.js` and no longer use `sort-<field_name>` tables as data source
- modify `scale.domain()` to include DataRef’s `.sort` object
- modify `sort` property to be consistent with Vega 2 (but still leave it as a top-level vega-lite property)
  - `sort.name` => `sort.field`
  - `sort.aggregate` => `sort.op`

- update tests


Patch:
diff --git a/gallery/gallery.js b/gallery/gallery.js
index 4abdb07ba..2e1af9dd1 100644
--- a/gallery/gallery.js
+++ b/gallery/gallery.js
@@ -10,8 +10,8 @@ var EXAMPLES = [
       data: {
         values: [
           {'x':'A', 'y':28}, {'x':'B', 'y':55}, {'x':'C', 'y':43},
-          {'x':'D', 'y':91}, {'x':'E', 'y':81}, {'x':'F', 'y':53},
-          {'x':'G', 'y':19}, {'x':'H', 'y':87}, {'x':'I', 'y':52}
+          {'x':'G', 'y':19}, {'x':'H', 'y':87}, {'x':'I', 'y':52},
+          {'x':'D', 'y':91}, {'x':'E', 'y':81}, {'x':'F', 'y':53}
         ]
       },
       marktype: 'bar',
@@ -69,9 +69,9 @@ var EXAMPLES = [
       data: {url: 'data/barley.json'},
       marktype: 'point',
       encoding: {
-        x: {type: 'Q',name: 'yield',aggregate: 'avg'},
+        x: {type: 'Q',name: 'yield', aggregate: 'mean'},
         y: {
-          sort: [{name: 'yield',aggregate: 'avg',reverse: false}],
+          sort: {field: 'yield', op: 'mean'},
           type: 'O',
           name: 'variety'
         },
diff --git a/src/compiler/compiler.js b/src/compiler/compiler.js
index 627a80ef1..37dec6607 100644
--- a/src/compiler/compiler.js
+++ b/src/compiler/compiler.js
@@ -18,7 +18,6 @@ var Encoding = require('../Encoding'),
 compiler.data = require('./data');
 compiler.facet = require('./facet');
 compiler.layout = require('./layout');
-compiler.sort = require('./sort');
 compiler.stack = require('./stack');
 compiler.style = require('./style');
 compiler.subfacet = require('./subfacet');
@@ -76,9 +75,6 @@ compiler.compileEncoding = function (encoding, stats) {
 
   var group = spec.marks[0];
 
-  // FIXME remove compiler.sort after migrating to vega 2.
-  spec.data = compiler.sort(spec.data, encoding, stats); // append new data
-
   // marks
   var style = compiler.style(encoding, stats),
     mdefs = group.marks = marks.def(encoding, layout, style, stats),
diff --git a/src/compiler/scale.js b/src/compiler/scale.js
index ca2ed41ae..af92938eb 100644
--- a/src/compiler/scale.js
+++ b/src/compiler/scale.js
@@ -4,8 +4,7 @@ var util = require('../util'),
   time = require('./time'),
   colorbrewer = require('colorbrewer'),
   interpolate = require('d3-color').interpolateHsl,
-  schema = require('../schema/schema'),
-  vlsort = require('./sort');
+  schema = require('../schema/schema');
 
 var scale = module.exports = {};
 
@@ -22,25 +21,16 @@ scale.defs = function(names, encoding, layout, stats, opt) {
   return names.reduce(function(a, name) {
     var s = {
       name: name,
-      type: scale.type(name, encoding),
-      domain: scale.domain(name, encoding, stats, opt)
+      type: scale.type(name, encoding)
     };
 
-    s.sort = scale.sort(s, encoding, name) || undefined;
-
+    s.domain = scale.domain(s, encoding, stats, opt);
     scale.range(s, encoding, layout, stats, opt);
 
     return (a.push(s), a);
   }, []);
 };
 
-scale.sort = function(s, encoding, name) {
-  return s.type === 'ordinal' && (
-    !!encoding.bin(name) ||
-    encoding.sort(name).length === 0
-  );
-};
-
 scale.type = function(name, encoding) {
 
   switch (encoding.type(name)) {
@@ -51,13 +41,18 @@ scale.type = function(name, encoding) {
       return timeUnit ? time.scale.type(timeUnit, name) : 'time';
     case Q:
       if (encoding.bin(name)) {
+        // TODO: revise this
         return name === COLOR ? 'linear' : 'ordinal';
       }
       return encoding.scale(name).type;
   }
 };
 
-scale.domain = function (name, encoding, stats, opt) {
+scale.domain = function (s, encoding, stats, opt) {
+  opt = opt || {};
+
+  var name = s.name;
+
   var field = encoding.field(name);
 
   if (encoding.isType(name, T)) {
@@ -90,8 +85,7 @@ scale.domain = function (name, encoding, stats, opt) {
       scaleUseRawDomain : encoding.config('useRawDomain'),
     notCountOrSum = !aggregate || (aggregate !=='count' && aggregate !== 'sum');
 
-  // FIXME revise this part
-
+  var dataTable = encoding.dataTable();
   if ( useRawDomain && notCountOrSum && (
       // Q always uses non-ordinal scale except when it's binned and thus uses ordinal scale.
       (encoding.isType(name, Q) && !field.bin) ||
@@ -99,15 +93,19 @@ scale.domain = function (name, encoding, stats, opt) {
       (encoding.isType(name, T) && (!timeUnit || !time.isOrdinalFn(timeUnit)))
     )
   ) {
-
-    return {data: RAW, field: encoding.fieldRef(name)};
+    dataTable = RAW;
   }
 
-  var data = encoding.sort(name, stats).length > 0 ?
-    vlsort.getDataName(name):
-    encoding.dataTable();
+  var domain = {
+    data: dataTable,
+    field: encoding.fieldRef(name)
+  };
+
+  // For ordinal scale, add domain's property if provided.
+  var sort = s.type === 'ordinal' && encoding.sort(name);
+  if (sort) { domain.sort = sort; }
 
-  return {data: data, field: encoding.fieldRef(name)};
+  return domain;
 };
 
 
diff --git a/src/compiler/sort.js b/src/compiler/sort.js
deleted file mode 100644
index 45ac7dfff..000000000
--- a/src/compiler/sort.js
+++ /dev/null
@@ -1,60 +0,0 @@
-'use strict';
-
-require('../globals');
-
-var vlfield = require('../field');
-
-module.exports = sort;
-
-// adds new transforms that produce sorted fields
-function sort(data, encoding, stats, opt) {
-  // jshint unused:false
-
-  var datasetMapping = {};
-
-  encoding.forEach(function(field, encType) {
-    var sortBy = encoding.sort(encType, stats);
-    if (sortBy.length > 0) {
-      var fields = sortBy.map(function(d) {
-        return {
-          op: d.aggregate,
-          field: vlfield.fieldRef(d, {nofn: true})
-        };
-      });
-
-      var byClause = sortBy.map(function(d) {
-        var reverse = (d.reverse ? '-' : '');
-        return reverse + vlfield.fieldRef(d);
-      });
-
-      var dataName = sort.getDataName(encType);
-
-      var transforms = [
-        {
-          type: 'aggregate',
-          groupby: [ encoding.fieldRef(encType) ],
-          fields: fields
-        },
-        {
-          type: 'sort',
-          by: byClause
-        }
-      ];
-
-      data.push({
-        name: dataName,
-        source: RAW,
-        transform: transforms
-      });
-
-      datasetMapping[encType] = dataName;
-    }
-  });
-
-  return data;
-}
-
-sort.getDataName = function(encType) {
-  return 'sorted-' + encType;
-};
-
diff --git a/src/schema/schema.js b/src/schema/schema.js
index 59d0db7df..46573cf73 100644
--- a/src/schema/schema.js
+++ b/src/schema/schema.js
@@ -225,24 +225,19 @@ var sortMixin = {
   type: 'object',
   properties: {
     sort: {
-      type: 'array',
-      default: [],
-      items: {
-        type: 'object',
-        supportedTypes: toMap([N, O]),
-        required: ['name', 'aggregate'],
-        properties: {
-          name: {
-            type: 'string'
-          },
-          aggregate: {
-            type: 'string',
-            enum: ['avg', 'sum', 'min', 'max', 'count']
-          },
-          reverse: {
-            type: 'boolean',
-            default: false
-          }
+      type: ['object', 'boolean'],
+      default: true,
+      supportedTypes: toMap([N, O]),
+      required: ['field', 'op'],
+      properties: {
+        field: {
+          type: 'string',
+          description: 'The field name to aggregate over.'
+        },
+        op: {
+          type: 'string',
+          enum: ['avg', 'sum', 'min', 'max', 'count'],
+          description: 'The field name to aggregate over.'
         }
       }
     }


Test patch:
diff --git a/test/compiler/scale.spec.js b/test/compiler/scale.spec.js
index 57a40838b..3d67aa421 100644
--- a/test/compiler/scale.spec.js
+++ b/test/compiler/scale.spec.js
@@ -9,173 +9,208 @@ var util = require('../../src/util'),
   colorbrewer = require('colorbrewer');
 
 describe('vl.compile.scale', function() {
-  describe('sort()', function() {
-    it('should return true for any ordinal or binned field', function() {
-      var encoding = Encoding.fromSpec({
+  describe('domain()', function() {
+    var linearScaleDef = {
+      name: 'y',
+      type: 'linear'
+    };
+
+    var ordinalScaleDef = {
+      name: 'y',
+      type: 'ordinal'
+    };
+    var timeScaleDef = {
+      name: 'y',
+      type: 'time'
+    };
+
+    describe('for stack', function() {
+      it('should return correct stack', function() {
+        var domain = vlscale.domain(linearScaleDef, Encoding.fromSpec({
           encoding: {
-            x: { name: 'origin', type: O},
-            y: { bin: true, name: 'origin', type: Q}
+            y: {
+              name: 'origin'
+            }
           }
+        }), {}, {
+          stack: 'y',
+          facet: true
         });
 
-      expect(vlscale.sort({type: 'ordinal'}, encoding, 'x'))
-        .to.eql(true);
-      expect(vlscale.sort({type: 'ordinal'}, encoding, 'y'))
-        .to.eql(true);
-    });
-
-  });
+        expect(domain).to.eql({
+          data: 'stacked',
+          field: 'max_sum_origin'
+        });
+      });
 
-  describe('domain()', function() {
-    it('should return correct stack', function() {
-      var domain = vlscale.domain('y', Encoding.fromSpec({
-        encoding: {
-          y: {
-            name: 'origin'
+      it('should return correct aggregated stack', function() {
+        var domain = vlscale.domain(linearScaleDef, Encoding.fromSpec({
+          encoding: {
+            y: {
+              aggregate: 'sum',
+              name: 'origin'
+            }
           }
-        }
-      }), {}, {
-        stack: 'y',
-        facet: true
-      });
+        }), {}, {
+          stack: 'y',
+          facet: true
+        });
 
-      expect(domain).to.eql({
-        data: 'stacked',
-        field: 'max_sum_origin'
+        expect(domain).to.eql({
+          data: 'stacked',
+          field: 'max_sum_sum_origin'
+        });
       });
     });
 
-    it('should return correct aggregated stack', function() {
-      var domain = vlscale.domain('y', Encoding.fromSpec({
-        encoding: {
-          y: {
-            aggregate: 'sum',
-            name: 'origin'
-          }
-        }
-      }), {}, {
-        stack: 'y',
-        facet: true
-      });
+    describe('for quantitative', function() {
+      it('should return the right domain if binned Q',
+        function() {
+          var domain = vlscale.domain(ordinalScaleDef, Encoding.fromSpec({
+            encoding: {
+              y: {
+                bin: {maxbins: 15},
+                name: 'origin',
+                scale: {useRawDomain: true},
+                type: Q
+              }
+            }
+          }), {origin: {min: -5, max:48}}, {});
 
-      expect(domain).to.eql({
-        data: 'stacked',
-        field: 'max_sum_sum_origin'
-      });
-    });
+          expect(domain).to.eql([-5, 0, 5, 10, 15, 20, 25, 30, 35, 40, 45]);
+        });
 
-    it('should return the right domain if binned Q',
-      function() {
-        var domain = vlscale.domain('y', Encoding.fromSpec({
-          encoding: {
-            y: {
-              bin: {maxbins: 15},
-              name: 'origin',
-              scale: {useRawDomain: true},
-              type: Q
+      it('should return the raw domain if useRawDomain is true for non-bin, non-sum Q',
+        function() {
+          var domain = vlscale.domain(linearScaleDef, Encoding.fromSpec({
+            encoding: {
+              y: {
+                aggregate: 'mean',
+                name: 'origin',
+                scale: {useRawDomain: true},
+                type: Q
+              }
             }
-          }
-        }), {origin: {min: -5, max:48}}, {});
+          }), {}, {});
 
-        expect(domain).to.eql([-5, 0, 5, 10, 15, 20, 25, 30, 35, 40, 45]);
-      });
+          expect(domain.data).to.eql(RAW);
+        });
 
-    it('should return the raw domain if useRawDomain is true for non-bin, non-sum Q',
-      function() {
-        var domain = vlscale.domain('y', Encoding.fromSpec({
-          encoding: {
-            y: {
-              aggregate: 'mean',
-              name: 'origin',
-              scale: {useRawDomain: true},
-              type: Q
+      it('should return the aggregate domain for sum Q',
+        function() {
+          var domain = vlscale.domain(linearScaleDef, Encoding.fromSpec({
+            encoding: {
+              y: {
+                aggregate: 'sum',
+                name: 'origin',
+                scale: {useRawDomain: true},
+                type: Q
+              }
             }
-          }
-        }), {}, {});
+          }), {}, {});
 
-        expect(domain.data).to.eql(RAW);
-      });
+          expect(domain.data).to.eql(AGGREGATE);
+        });
 
-    it('should return the aggregate domain for sum Q',
-      function() {
-        var domain = vlscale.domain('y', Encoding.fromSpec({
-          encoding: {
-            y: {
-              aggregate: 'sum',
-              name: 'origin',
-              scale: {useRawDomain: true},
-              type: Q
+
+      it('should return the aggregated domain if useRawDomain is false', function() {
+          var domain = vlscale.domain(linearScaleDef, Encoding.fromSpec({
+            encoding: {
+              y: {
+                aggregate: 'min',
+                name: 'origin',
+                scale: {useRawDomain: false},
+                type: Q
+              }
             }
-          }
-        }), {}, {});
+          }), {}, {});
 
-        expect(domain.data).to.eql(AGGREGATE);
-      });
+          expect(domain.data).to.eql(AGGREGATE);
+        });
+    });
 
-    it('should return the raw domain if useRawDomain is true for raw T',
-      function() {
-        var domain = vlscale.domain('y', Encoding.fromSpec({
-          encoding: {
-            y: {
-              name: 'origin',
-              scale: {useRawDomain: true},
-              type: T
+    describe('for time', function() {
+      it('should return the raw domain if useRawDomain is true for raw T',
+        function() {
+          var domain = vlscale.domain(timeScaleDef, Encoding.fromSpec({
+            encoding: {
+              y: {
+                name: 'origin',
+                scale: {useRawDomain: true},
+                type: T
+              }
             }
-          }
-        }), {}, {});
+          }), {}, {});
 
-        expect(domain.data).to.eql(RAW);
-      });
+          expect(domain.data).to.eql(RAW);
+        });
 
-    it('should return the raw domain if useRawDomain is true for year T',
-      function() {
-        var domain = vlscale.domain('y', Encoding.fromSpec({
-          encoding: {
-            y: {
-              name: 'origin',
-              scale: {useRawDomain: true},
-              type: T,
-              timeUnit: 'year'
+      it('should return the raw domain if useRawDomain is true for year T',
+        function() {
+          var domain = vlscale.domain(ordinalScaleDef, Encoding.fromSpec({
+            encoding: {
+              y: {
+                name: 'origin',
+                scale: {useRawDomain: true},
+                type: T,
+                timeUnit: 'year'
+              }
             }
-          }
-        }), {}, {});
+          }), {}, {});
 
-        expect(domain.data).to.eql(RAW);
-        expect(domain.field.indexOf('year')).to.gt(-1);
-      });
+          expect(domain.data).to.eql(RAW);
+          expect(domain.field.indexOf('year')).to.gt(-1);
+        });
 
-    it('should return the correct domain for month T',
-      function() {
-        var domain = vlscale.domain('y', Encoding.fromSpec({
-          encoding: {
-            y: {
-              name: 'origin',
-              scale: {useRawDomain: true},
-              type: T,
-              timeUnit: 'month'
+      it('should return the correct domain for month T',
+        function() {
+          var domain = vlscale.domain(ordinalScaleDef, Encoding.fromSpec({
+            encoding: {
+              y: {
+                name: 'origin',
+                scale: {useRawDomain: true},
+                type: T,
+                timeUnit: 'month'
+              }
             }
-          }
-        }), {}, {});
+          }), {}, {});
+
+          expect(domain).to.eql([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]);
+        });
+    });
+
+    describe('for ordinal', function() {
+      it('should return correct domain with the provided sort property', function() {
+        var sortDef = {aggregate: 'min', field:'Acceleration'};
+        var encoding = Encoding.fromSpec({
+            encoding: {
+              y: { name: 'origin', type: O, sort: sortDef}
+            }
+          });
 
-        expect(domain).to.eql([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]);
+        expect(vlscale.domain(ordinalScaleDef, encoding))
+          .to.eql({
+            data: RAW,
+            field: 'origin',
+            sort: sortDef
+          });
       });
 
-    it('should return the aggregated domain if useRawDomain is false', function() {
-      var domain = vlscale.domain('y', Encoding.fromSpec({
-        encoding: {
-          y: {
-            aggregate: 'min',
-            name: 'origin',
-            scale: {useRawDomain: false},
-            type: Q
-          }
-        }
-      }), {}, {});
+      it('should return correct domain with sort=true if sort is not provided', function() {
+        var encoding = Encoding.fromSpec({
+            encoding: {
+              y: { name: 'origin', type: O}
+            }
+          });
 
-      expect(domain.data).to.eql(AGGREGATE);
+        expect(vlscale.domain(ordinalScaleDef, encoding))
+          .to.eql({
+            data: RAW,
+            field: 'origin',
+            sort: true
+          });
+      });
     });
-
-    // TODO test other cases
   });
 
   describe('color.palette', function() {
diff --git a/test/compiler/sort.spec.js b/test/compiler/sort.spec.js
deleted file mode 100644
index 14740c20b..000000000
--- a/test/compiler/sort.spec.js
+++ /dev/null
@@ -1,56 +0,0 @@
-'use strict';
-
-var expect = require('chai').expect;
-
-var vlsort = require('../../src/compiler/sort'),
-  Encoding = require('../../src/Encoding');
-
-describe('Sort', function() {
-  var encoding = Encoding.fromSpec({
-        encoding: {
-          x: {name: 'foo', type: 'O', sort: [{
-            name: 'bar', aggregate: 'avg'
-          }]},
-          y: {name: 'bar', type: 'Q'},
-          color: {name: 'baz', type: 'O', sort: [{
-            name: 'bar', aggregate: 'sum'
-          }, {
-            name: 'foo', aggregate: 'max', reverse: true
-          }]}
-        }
-      }),
-    data = [{name: RAW}, {name: AGGREGATE}];
-
-  vlsort(data, encoding, {});
-
-  it('should add new data and transform', function() {
-    expect(data.length).to.equal(4);
-
-    expect(data[2].transform).to.deep.equal([
-      {
-        type: 'aggregate',
-        groupby: [ 'foo' ],
-        fields: [{
-          field: 'bar',
-          op: 'avg'
-        }]
-      },
-      { type: 'sort', by: [ 'avg_bar' ] }
-    ]);
-
-    expect(data[3].transform).to.deep.equal([
-      {
-        type: 'aggregate',
-        groupby: [ 'baz' ],
-        fields: [{
-          field: 'bar',
-          op: 'sum'
-        }, {
-          field: 'foo',
-          op: 'max'
-        }]
-      },
-      { type: 'sort', by: [ 'sum_bar', '-max_foo' ] }
-    ]);
-  });
-});