Repository: vega/vega-lite
Pull request #672 (2015-10-18 06:27:10)

Refactor
See 84595dc for branch specific diff.  

__Please merge #671 first!__


Patch:
diff --git a/docs/Encoding-Mapping.md b/docs/Encoding-Mapping.md
index 5c4bc8517..ddcdc6477 100644
--- a/docs/Encoding-Mapping.md
+++ b/docs/Encoding-Mapping.md
@@ -20,14 +20,14 @@ Here are the list of common properties of the encoding property definition objec
 | name          | String        | A field/variable from which to pull a data value.  __<sup>1</sup>__  |
 | value         | String,Integer |                                            |
 | type          | String        | Data Type (`Q` for quantitative, `O` for ordinal, `T` for temporal, and `N` for nominal).  __<sup>2</sup>__ |
-| [axis](#axis)           | Object        | Configuration object for the encoding's axis    |
-| [legends](#legends)     | Object        | Configuration object for the encoding's legends |
-| [scale](#scale)         | Object        | Configuration object for the encoding's scale   |
-| [sort](#sort)           | Object        | Configuration object for the encoding's order   |
+| [axis](#axis)        | Object        | Configuration object for the encoding's axis    |
+| [legends](#legends)  | Object        | Configuration object for the encoding's legends |
+| [scale](#scale)      | Object        | Configuration object for the encoding's scale   |
+| [sort](#sort)        | String \| Object        | For all types of fields, if specified to `ascending` or `descending`, the domain values are sorted in based on the field's value in ascending or descending order. For nominal and ordinal fields, if set to an object, the values in the scale domain will be sorted based on an aggregate calculation over a specified sort field.  <!--  TODO say what happen when sort is unspecified. -->  See [Sort](#sort) section for more information.  |
 | [aggregate](#aggregate) | String        | Aggregation function for the field (`mean`, `sum`, `median`, `min`, `max`, `count`)  |
 
-| [bin](#bin)             | Object        | Binning properties.  See [Binning](#Binning) |
-| [timeUnit](#timeunit)   | String        | Property for converting time unit            |
+| [bin](#bin)          | Object        | Binning properties.  See [Binning](#Binning) |
+| [timeUnit](#timeunit)| String        | Property for converting time unit            |
 
 
 __<sup>1</sup>__ __Pending Revision__
@@ -71,10 +71,10 @@ If none of the specified encoding channel contains aggregation, no additional da
 Vega-lite's `scale` object supports the following Vega scale properties:
 
 
-- [Vega Common Scale Properties](https://github.com/vega/vega/wiki/Scales#common-scale-properties)__<sup>2</sup>__: `type`__<sup>1</sup>__ and `reverse`
+- [Vega Common Scale Properties](https://github.com/vega/vega/wiki/Scales#common-scale-properties)__<sup>2</sup>__: `type`__<sup>1,2</sup>__.
 
 
-- [Vega Quantitative Scale Properties](https://github.com/vega/vega/wiki/Scales#quantitative-scale-properties)__<sup>2</sup>__: `nice` and `zero`
+- [Vega Quantitative Scale Properties](https://github.com/vega/vega/wiki/Scales#quantitative-scale-properties)__<sup>3</sup>__: `nice` and `zero`
 
 
 See [Vega's documentation](https://github.com/vega/vega/wiki/Scales#common-scale-properties) for more information about these properties.
@@ -86,11 +86,13 @@ Moreover, Vega-lite has the following additional scale properties:
 | :------------ |:-------------:| :------------- |
 | useRawDomain  | Boolean       | Use the raw data instead of summary data for scale domain (Only for aggregated field).  Note that this option does not work with sum or count aggregate as they could have a substantially larger scale range. |
 
-__<sup>1</sup>__ __In Roadmap__:
-Other applicable Vega scale properties will be added. [#181](../../issues/181)
 
+__<sup>1</sup>__ `reverse` is excluded from Vega-lite's `scale` to avoid conflicting with `sort` property.  Please use `sort='descending'` instead.
+
+__<sup>2</sup>__ __In Roadmap__:
+Other applicable Vega scale properties will be added. [#181](../../issues/181)
 
-__<sup>2</sup>__
+__<sup>3</sup>__
 Vega-lite automatically determines scale's `type` based on the field's data type.
 By default, scales of nominal and ordinal fields are ordinal scales.
 Scales of time fields are time scales if time unit conversion is not applied.
@@ -135,7 +137,20 @@ For now please see [legends json schema in schema.js](https://github.com/uwdata/
 
 ## sort
 
-_(Coming Soon)_
+`sort` property can be specify for sorting the field's values in multiple ways:
+
+- `undefined` - the field is unsorted.
+- (as __String__) `'ascending'` or `'descending'` – the field is sort by the field's value in ascending or descending order.
+- (as __Object__) A sort field object - for sorting the field by an aggregate calculation over a specified sort field.  A sort field object has the following properties:
+
+| Property      | Type          | Description    |
+| :------------ |:-------------:| :------------- |
+| _sort.field_  | Field         | The field name to aggregate over.|
+| _sort.op_     | String        | A valid [aggregation operation](Data-Transforms#-aggregate) (e.g., `mean`, `median`, etc.).|
+| _sort.order_  | String        | (Optional) `'ascending'` or `'descending'` order. |
+
+
+
 
 # Channel Specific Properties
 
diff --git a/src/compiler/scale.js b/src/compiler/scale.js
index 64d144830..6026ae9db 100644
--- a/src/compiler/scale.js
+++ b/src/compiler/scale.js
@@ -17,12 +17,18 @@ scale.names = function(props) {
 
 scale.defs = function(names, encoding, layout, stats, facet) {
   return names.reduce(function(a, name) {
-    var scaleDef = {
-      name: name,
-      type: scale.type(name, encoding)
-    };
+    var scaleDef = {};
+
+    scaleDef.name = name;
+    scaleDef.type = scale.type(name, encoding);
+    scaleDef.domain = scale.domain(encoding, name, scaleDef.type, stats, facet);
+
+    // add `reverse` if applicable
+    var reverse = scale.reverse(encoding, name);
+    if (reverse !== undefined) {
+      scaleDef.reverse = reverse;
+    }
 
-    scaleDef.domain = scale.domain(scaleDef, encoding, stats, facet);
     scaleDef = scale.range(scaleDef, encoding, layout, stats);
 
     return (a.push(scaleDef), a);
@@ -30,7 +36,6 @@ scale.defs = function(names, encoding, layout, stats, facet) {
 };
 
 scale.type = function(name, encoding) {
-
   switch (encoding.type(name)) {
     case N: //fall through
     case O: return 'ordinal';
@@ -46,9 +51,7 @@ scale.type = function(name, encoding) {
   }
 };
 
-scale.domain = function (scaleDef, encoding, stats, facet) {
-  var name = scaleDef.name;
-
+scale.domain = function (encoding, name, type, stats, facet) {
   var encDef = encoding.encDef(name);
 
   // special case for temporal scale
@@ -90,13 +93,36 @@ scale.domain = function (scaleDef, encoding, stats, facet) {
       field: encoding.fieldRef(name)
     };
 
-  // For ordinal scale, add domain's property if provided.
-  var sort = scaleDef.type === 'ordinal' && encoding.sort(name);
-  if (sort) { domain.sort = sort; }
+  // Add `sort` if applicable
+  var sort = scale.sort(encoding, name, type);
+  if (sort) {
+    domain.sort = sort;
+  }
 
   return domain;
 };
 
+scale.sort = function(encoding, name, type) {
+  var sort = encoding.encDef(name).sort;
+  if (sort === 'ascending' || sort === 'descending') {
+    return true;
+  }
+
+  // Sorted based on an aggregate calculation over a specified sort field (only for ordinal scale)
+  if (type === 'ordinal' && util.isObject(sort)) {
+    return {
+      op: sort.op,
+      field: sort.field
+    };
+  }
+  return undefined;
+};
+
+scale.reverse = function(encoding, name) {
+  var sort = encoding.sort(name);
+  return sort && (sort === 'descending' || (sort.order === 'descending'));
+};
+
 /**
  * Determine if useRawDomain should be activated for this scale.
  * @return {Boolean} Returns true if all of the following conditons applies:
@@ -151,8 +177,6 @@ scale.range = function (scaleDef, encoding, layout, stats) {
         } else {
           scaleDef.zero = spec.zero === undefined ? true : spec.zero;
         }
-
-        scaleDef.reverse = spec.reverse;
       }
       scaleDef.round = true;
       if (scaleDef.type === 'time') {
@@ -174,8 +198,6 @@ scale.range = function (scaleDef, encoding, layout, stats) {
         } else {
           scaleDef.zero = spec.zero === undefined ? true : spec.zero;
         }
-
-        scaleDef.reverse = spec.reverse;
       }
 
       scaleDef.round = true;
diff --git a/src/schema/schema.js b/src/schema/schema.js
index 906eeeb85..8bd72ea2b 100644
--- a/src/schema/schema.js
+++ b/src/schema/schema.js
@@ -99,11 +99,6 @@ var typicalField = merge(clone(schema.field), {
       properties: {
         /* Common Scale Properties */
         type: schema.scale_type,
-        reverse: {
-          type: 'boolean',
-          default: false,
-          supportedTypes: toMap([Q, T])
-        },
 
         /* Quantitative Scale Properties */
         nice: {
@@ -231,21 +226,34 @@ var sortMixin = {
   type: 'object',
   properties: {
     sort: {
-      type: ['object', 'boolean'],
-      default: true,
+      default: undefined,
       supportedTypes: toMap([N, O]),
-      required: ['field', 'op'],
-      properties: {
-        field: {
+      oneOf: [
+        {
           type: 'string',
-          description: 'The field name to aggregate over.'
+          enum: ['ascending', 'descending']
         },
-        op: {
-          type: 'string',
-          enum: VALID_AGG_OPS,
-          description: 'The field name to aggregate over.'
+        { // sort by aggregation of another field
+          type: 'object',
+          required: ['field', 'op'],
+          properties: {
+            field: {
+              type: 'string',
+              description: 'The field name to aggregate over.'
+            },
+            op: {
+              type: 'string',
+              enum: VALID_AGG_OPS,
+              description: 'The field name to aggregate over.'
+            },
+            order: {
+              type: 'string',
+              enum: ['ascending', 'descending']
+            }
+          }
         }
-      }
+      ]
+
     }
   }
 };


Test patch:
diff --git a/test/compiler/scale.spec.js b/test/compiler/scale.spec.js
index 8229ac69b..983e2d8c7 100644
--- a/test/compiler/scale.spec.js
+++ b/test/compiler/scale.spec.js
@@ -10,23 +10,9 @@ var util = require('../../src/util'),
 
 describe('vl.compile.scale', function() {
   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({
+        var domain = vlscale.domain(Encoding.fromSpec({
           marktype: 'bar',
           encoding: {
             y: {
@@ -36,7 +22,7 @@ describe('vl.compile.scale', function() {
             x: {name: 'x', type: 'O'},
             color: {name: 'color', type: 'O'}
           }
-        }), {}, true);
+        }), 'y', 'linear', {}, true);
 
         expect(domain).to.eql({
           data: 'stacked',
@@ -45,7 +31,7 @@ describe('vl.compile.scale', function() {
       });
 
       it('should return correct aggregated stack', function() {
-        var domain = vlscale.domain(linearScaleDef, Encoding.fromSpec({
+        var domain = vlscale.domain(Encoding.fromSpec({
           marktype: 'bar',
           encoding: {
             y: {
@@ -55,7 +41,7 @@ describe('vl.compile.scale', function() {
             x: {name: 'x', type: 'O'},
             color: {name: 'color', type: 'O'}
           }
-        }), {}, true);
+        }), 'y', 'linear', {}, true);
 
         expect(domain).to.eql({
           data: 'stacked',
@@ -67,7 +53,7 @@ describe('vl.compile.scale', function() {
     describe('for quantitative', function() {
       it('should return the right domain if binned Q',
         function() {
-          var domain = vlscale.domain(ordinalScaleDef, Encoding.fromSpec({
+          var domain = vlscale.domain(Encoding.fromSpec({
             encoding: {
               y: {
                 bin: {maxbins: 15},
@@ -76,14 +62,14 @@ describe('vl.compile.scale', function() {
                 type: Q
               }
             }
-          }), {origin: {min: -5, max:48}}, {});
+          }), 'y', 'ordinal', {origin: {min: -5, max:48}}, {});
 
           expect(domain).to.eql([-5, 0, 5, 10, 15, 20, 25, 30, 35, 40, 45]);
         });
 
       it('should return the raw domain if useRawDomain is true for non-bin, non-sum Q',
         function() {
-          var domain = vlscale.domain(linearScaleDef, Encoding.fromSpec({
+          var domain = vlscale.domain(Encoding.fromSpec({
             encoding: {
               y: {
                 aggregate: 'mean',
@@ -92,14 +78,14 @@ describe('vl.compile.scale', function() {
                 type: Q
               }
             }
-          }), {}, {});
+          }), 'y', 'linear', {}, {});
 
           expect(domain.data).to.eql(RAW);
         });
 
       it('should return the aggregate domain for sum Q',
         function() {
-          var domain = vlscale.domain(linearScaleDef, Encoding.fromSpec({
+          var domain = vlscale.domain(Encoding.fromSpec({
             encoding: {
               y: {
                 aggregate: 'sum',
@@ -108,14 +94,14 @@ describe('vl.compile.scale', function() {
                 type: Q
               }
             }
-          }), {}, {});
+          }), 'y', 'linear', {}, {});
 
           expect(domain.data).to.eql(AGGREGATE);
         });
 
 
       it('should return the aggregated domain if useRawDomain is false', function() {
-          var domain = vlscale.domain(linearScaleDef, Encoding.fromSpec({
+          var domain = vlscale.domain(Encoding.fromSpec({
             encoding: {
               y: {
                 aggregate: 'min',
@@ -124,7 +110,7 @@ describe('vl.compile.scale', function() {
                 type: Q
               }
             }
-          }), {}, {});
+          }), 'y', 'linear', {}, {});
 
           expect(domain.data).to.eql(AGGREGATE);
         });
@@ -133,7 +119,7 @@ describe('vl.compile.scale', function() {
     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({
+          var domain = vlscale.domain(Encoding.fromSpec({
             encoding: {
               y: {
                 name: 'origin',
@@ -141,14 +127,14 @@ describe('vl.compile.scale', function() {
                 type: T
               }
             }
-          }), {}, {});
+          }), 'y', 'time', {}, {});
 
           expect(domain.data).to.eql(RAW);
         });
 
       it('should return the raw domain if useRawDomain is true for year T',
         function() {
-          var domain = vlscale.domain(ordinalScaleDef, Encoding.fromSpec({
+          var domain = vlscale.domain(Encoding.fromSpec({
             encoding: {
               y: {
                 name: 'origin',
@@ -157,7 +143,7 @@ describe('vl.compile.scale', function() {
                 timeUnit: 'year'
               }
             }
-          }), {}, {});
+          }), 'y', 'ordinal', {}, {});
 
           expect(domain.data).to.eql(RAW);
           expect(domain.field.indexOf('year')).to.gt(-1);
@@ -165,7 +151,7 @@ describe('vl.compile.scale', function() {
 
       it('should return the correct domain for month T',
         function() {
-          var domain = vlscale.domain(ordinalScaleDef, Encoding.fromSpec({
+          var domain = vlscale.domain(Encoding.fromSpec({
             encoding: {
               y: {
                 name: 'origin',
@@ -174,7 +160,7 @@ describe('vl.compile.scale', function() {
                 timeUnit: 'month'
               }
             }
-          }), {}, {});
+          }), 'y', 'ordinal', {}, {});
 
           expect(domain).to.eql([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]);
         });
@@ -182,14 +168,14 @@ describe('vl.compile.scale', function() {
 
     describe('for ordinal', function() {
       it('should return correct domain with the provided sort property', function() {
-        var sortDef = {aggregate: 'min', field:'Acceleration'};
+        var sortDef = {op: 'min', field:'Acceleration'};
         var encoding = Encoding.fromSpec({
             encoding: {
               y: { name: 'origin', type: O, sort: sortDef}
             }
           });
 
-        expect(vlscale.domain(ordinalScaleDef, encoding))
+        expect(vlscale.domain(encoding, 'y', 'ordinal'))
           .to.eql({
             data: RAW,
             field: 'origin',
@@ -197,18 +183,17 @@ describe('vl.compile.scale', function() {
           });
       });
 
-      it('should return correct domain with sort=true if sort is not provided', function() {
+      it('should return correct domain without sort if sort is not provided', function() {
         var encoding = Encoding.fromSpec({
             encoding: {
               y: { name: 'origin', type: O}
             }
           });
 
-        expect(vlscale.domain(ordinalScaleDef, encoding))
+        expect(vlscale.domain(encoding, 'y', 'ordinal'))
           .to.eql({
             data: RAW,
-            field: 'origin',
-            sort: true
+            field: 'origin'
           });
       });
     });