Repository: vega/vega-lite (ts)

Issue:
Should we add `order` property to `encDef.sort`?
Currently Vega-lite’s `encDef.sort` looks exactly like vega Scale's `DataRef.sort`, which only contains `aggr` and `op` properties while the `order` field is deprecated as same mechanism can be achieved by using `reverse` property of the scale.  

However, in vega-lite’s encoding definition, `sort` is another top-level object, not a part of `scale`.  If we do not have `order` property.  Specifying descending sort would require specifying both `sort` and `scale` like the following Barley example, which is kind of miss the point for extracting `sort` from `scale`. 

```js
{
      data: {url: 'data/barley.json'},
      marktype: 'point',
      encoding: {
        x: {type: 'Q',name: 'yield'},
        y: {
          name: 'variety',
          type: 'O',
          sort: {field: 'yield', op: 'median'},
          __scale: {reverse: true}__
        },
        row: {
          type: 'O',
          name: 'site',
          sort: {field: 'yield', op: 'median'},
          __scale: {reverse: true}__
        },
        color: {type: 'N',name: 'year'}
      }
    }
``` 

By having `order` property, the spec can be a bit nicer:

```
{
  data: {url: 'data/barley.json'},
  marktype: 'point',
  encoding: {
    x: {type: 'Q',name: 'yield'},
    y: {
      name: 'variety',
      type: 'O',
      sort: {field: 'yield', op: ‘median’, __order:’desc'__}
    },
    row: {
      type: 'O',
      name: 'site',
      sort: {field: 'yield', op: 'median', __order:’desc’__},
    },
    color: {type: 'N',name: 'year'}
  }
}
```


Pull request description:
Make `sort` support `undefined`, String (for all types), or Object (for ordinal/nominal only) 
Fixes #663 

## Change List

- now `sort` property can be either `undefined`, a __String__ or an __Object__
  - `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__ — for ordinal/nominal fields only) A sort field object - for sorting the field by an aggregate calculation (`sort.op`) over a specified sort field (`sort.field`) in a specified order (`sort.order`)

## Internal 

- add `scale.sort` that correct determine `sort` output
- add `scale.reverse` and determine the value from `sort`
- update docs regarding `sort`

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..8d6fc9329 100644
--- a/src/compiler/scale.js
+++ b/src/compiler/scale.js
@@ -23,6 +23,12 @@ scale.defs = function(names, encoding, layout, stats, facet) {
     };
 
     scaleDef.domain = scale.domain(scaleDef, encoding, stats, facet);
+
+    var reverse = scale.reverse(encoding, name);
+    if (reverse !== undefined) {
+      scaleDef.reverse = reverse;
+    }
+
     scaleDef = scale.range(scaleDef, encoding, layout, stats);
 
     return (a.push(scaleDef), a);
@@ -46,6 +52,7 @@ scale.type = function(name, encoding) {
   }
 };
 
+// TODO: change scaleDef to name, type
 scale.domain = function (scaleDef, encoding, stats, facet) {
   var name = scaleDef.name;
 
@@ -90,13 +97,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, scaleDef.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.encDef(name).sort;
+  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 +181,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 +202,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..a79ef7742 100644
--- a/test/compiler/scale.spec.js
+++ b/test/compiler/scale.spec.js
@@ -182,7 +182,7 @@ 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}
@@ -197,7 +197,7 @@ 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}
@@ -207,8 +207,7 @@ describe('vl.compile.scale', function() {
         expect(vlscale.domain(ordinalScaleDef, encoding))
           .to.eql({
             data: RAW,
-            field: 'origin',
-            sort: true
+            field: 'origin'
           });
       });
     });