You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ma...@apache.org on 2018/09/10 17:28:19 UTC

[incubator-superset] branch master updated: Handle "ambiguous durations" (#5785)

This is an automated email from the ASF dual-hosted git repository.

maximebeauchemin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git


The following commit(s) were added to refs/heads/master by this push:
     new f740974  Handle "ambiguous durations" (#5785)
f740974 is described below

commit f740974bf72cf6f13c084cd890c17f2960f9fcde
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Mon Sep 10 10:28:17 2018 -0700

    Handle "ambiguous durations" (#5785)
    
    * WIP
    
    * Update interface
    
    * Fix truncate
    
    * Improve unit tests
    
    * Improve code
    
    * Use moment.js to parse ISO durations
    
    * Fix typo and React props
---
 superset/assets/package.json                       |   1 -
 .../assets/spec/javascripts/modules/time_spec.js   | 100 ++++++++++++--------
 superset/assets/src/modules/time.js                | 104 +++++++++++++++++----
 .../deckgl/AnimatableDeckGLContainer.jsx           |  11 +--
 .../deckgl/CategoricalDeckGLContainer.jsx          |   6 +-
 .../visualizations/deckgl/layers/screengrid.jsx    |   6 +-
 superset/assets/yarn.lock                          |   4 -
 7 files changed, 160 insertions(+), 72 deletions(-)

diff --git a/superset/assets/package.json b/superset/assets/package.json
index 137fa01..3a34335 100644
--- a/superset/assets/package.json
+++ b/superset/assets/package.json
@@ -84,7 +84,6 @@
     "mousetrap": "^1.6.1",
     "mustache": "^2.2.1",
     "nvd3": "1.8.6",
-    "parse-iso-duration": "^1.0.0",
     "prop-types": "^15.6.0",
     "re-resizable": "^4.3.1",
     "react": "^15.6.2",
diff --git a/superset/assets/spec/javascripts/modules/time_spec.js b/superset/assets/spec/javascripts/modules/time_spec.js
index 59dab8e..36260a4 100644
--- a/superset/assets/spec/javascripts/modules/time_spec.js
+++ b/superset/assets/spec/javascripts/modules/time_spec.js
@@ -1,6 +1,32 @@
 import { it, describe } from 'mocha';
 import { expect } from 'chai';
-import { getPlaySliderParams } from '../../../src/modules/time';
+
+import moment from 'moment';
+import { getPlaySliderParams, truncate } from '../../../src/modules/time';
+
+describe('truncate', () => {
+  it('truncates timestamps', () => {
+    const timestamp = moment('2018-03-03T03:03:03.333');
+    const isoDurations = [
+      // basic units
+      [moment.duration('PT1S'), moment('2018-03-03T03:03:03')],
+      [moment.duration('PT1M'), moment('2018-03-03T03:03:00')],
+      [moment.duration('PT1H'), moment('2018-03-03T03:00:00')],
+      [moment.duration('P1D'), moment('2018-03-03T00:00:00')],
+      [moment.duration('P1M'), moment('2018-03-01T00:00:00')],
+      [moment.duration('P1Y'), moment('2018-01-01T00:00:00')],
+
+      // durations that are multiples
+      [moment.duration('PT2H'), moment('2018-03-03T02:00:00')],
+      [moment.duration('P2D'), moment('2018-03-03T00:00:00')],
+    ];
+    let result;
+    isoDurations.forEach(([step, expected]) => {
+      result = truncate(timestamp, step);
+      expect(result.format()).to.equal(expected.format());
+    });
+  });
+});
 
 describe('getPlaySliderParams', () => {
   it('is a function', () => {
@@ -9,49 +35,49 @@ describe('getPlaySliderParams', () => {
 
   it('handles durations', () => {
     const timestamps = [
-      new Date('2018-01-01'),
-      new Date('2018-01-02'),
-      new Date('2018-01-03'),
-      new Date('2018-01-04'),
-      new Date('2018-01-05'),
-      new Date('2018-01-06'),
-      new Date('2018-01-07'),
-      new Date('2018-01-08'),
-      new Date('2018-01-09'),
-      new Date('2018-01-10'),
-    ].map(d => d.getTime());
-    const { start, end, step, values, disabled } = getPlaySliderParams(timestamps, 'P2D');
-    expect(new Date(start)).to.eql(new Date('2018-01-01'));
-    expect(new Date(end)).to.eql(new Date('2018-01-11'));
-    expect(step).to.equal(2 * 24 * 60 * 60 * 1000);
-    expect(values.map(v => new Date(v))).to.eql([
-      new Date('2018-01-01'),
-      new Date('2018-01-03'),
+      moment('2018-01-01T00:00:00'),
+      moment('2018-01-02T00:00:00'),
+      moment('2018-01-03T00:00:00'),
+      moment('2018-01-04T00:00:00'),
+      moment('2018-01-05T00:00:00'),
+      moment('2018-01-06T00:00:00'),
+      moment('2018-01-07T00:00:00'),
+      moment('2018-01-08T00:00:00'),
+      moment('2018-01-09T00:00:00'),
+      moment('2018-01-10T00:00:00'),
+    ].map(d => parseInt(d.format('x'), 10));
+    const { start, end, getStep, values, disabled } = getPlaySliderParams(timestamps, 'P2D');
+    expect(moment(start).format()).to.equal(moment('2018-01-01T00:00:00').format());
+    expect(moment(end).format()).to.equal(moment('2018-01-11T00:00:00').format());
+    expect(getStep(start)).to.equal(2 * 24 * 60 * 60 * 1000);
+    expect(values.map(v => moment(v).format())).to.eql([
+      moment('2018-01-01T00:00:00').format(),
+      moment('2018-01-03T00:00:00').format(),
     ]);
     expect(disabled).to.equal(false);
   });
 
   it('handles intervals', () => {
     const timestamps = [
-      new Date('2018-01-01'),
-      new Date('2018-01-02'),
-      new Date('2018-01-03'),
-      new Date('2018-01-04'),
-      new Date('2018-01-05'),
-      new Date('2018-01-06'),
-      new Date('2018-01-07'),
-      new Date('2018-01-08'),
-      new Date('2018-01-09'),
-      new Date('2018-01-10'),
-    ].map(d => d.getTime());
+      moment('2018-01-01T00:00:00'),
+      moment('2018-01-02T00:00:00'),
+      moment('2018-01-03T00:00:00'),
+      moment('2018-01-04T00:00:00'),
+      moment('2018-01-05T00:00:00'),
+      moment('2018-01-06T00:00:00'),
+      moment('2018-01-07T00:00:00'),
+      moment('2018-01-08T00:00:00'),
+      moment('2018-01-09T00:00:00'),
+      moment('2018-01-10T00:00:00'),
+    ].map(d => parseInt(d.format('x'), 10));
     // 1970-01-03 was a Saturday
-    const { start, end, step, values, disabled } = getPlaySliderParams(timestamps, 'P1W/1970-01-03T00:00:00Z');
-    expect(new Date(start)).to.eql(new Date('2017-12-30'));  // Saturday
-    expect(new Date(end)).to.eql(new Date('2018-01-13'));  // Saturday
-    expect(step).to.equal(7 * 24 * 60 * 60 * 1000);
-    expect(values.map(v => new Date(v))).to.eql([
-      new Date('2017-12-30'),
-      new Date('2018-01-06'),
+    const { start, end, getStep, values, disabled } = getPlaySliderParams(timestamps, 'P1W/1970-01-03T00:00:00Z');
+    expect(moment(start).format()).to.equal(moment('2017-12-30T00:00:00Z').format());  // Saturday
+    expect(moment(end).format()).to.equal(moment('2018-01-13T00:00:00Z').format());  // Saturday
+    expect(getStep(start)).to.equal(7 * 24 * 60 * 60 * 1000);
+    expect(values.map(v => moment(v).format())).to.eql([
+      moment('2017-12-30T00:00:00Z').format(),
+      moment('2018-01-06T00:00:00Z').format(),
     ]);
     expect(disabled).to.equal(false);
   });
diff --git a/superset/assets/src/modules/time.js b/superset/assets/src/modules/time.js
index 7ebc4d7..8b9437f 100644
--- a/superset/assets/src/modules/time.js
+++ b/superset/assets/src/modules/time.js
@@ -1,38 +1,106 @@
-import parseIsoDuration from 'parse-iso-duration';
+import moment from 'moment';
 
 
+// array with the minimum values of each part of a timestamp -- note that
+// months are zero-indexed in Javascript
+const truncatePartTo = [
+  1,  // year
+  0,  // month
+  1,  // day
+  0,  // hour
+  0,  // minute
+  0,  // second
+  0,  // millisecond
+];
+
+
+export function truncate(timestamp, step) {
+  /*
+   * Truncate timestamp down to duration resolution.
+   */
+  const lowerBound = moment(timestamp).subtract(step);
+  const explodedTimestamp = timestamp.toArray();
+  const explodedLowerBound = lowerBound.toArray();
+
+  const firstDiffIndex = explodedTimestamp
+    .map((part, i) => (explodedLowerBound[i] !== part))
+    .indexOf(true);
+  const dateParts = explodedTimestamp.map((part, i) => {
+    if (i === firstDiffIndex) {
+      // truncate down to closest `truncatePartTo[i] + n * step`
+      const difference = part - explodedLowerBound[i];
+      return part - ((part - truncatePartTo[i]) % difference);
+    } else if (i < firstDiffIndex || firstDiffIndex === -1) {
+      return part;
+    }
+    return truncatePartTo[i];
+  });
+
+  return moment(dateParts);
+}
+
+function getStepSeconds(step, start) {
+  /* Return number of seconds in a step.
+   *
+   * The step might be ambigous, eg, "1 month" has a variable number of
+   * seconds, which is why we need to know the start time.
+   */
+  const startMillliseconds = parseInt(moment(start).format('x'), 10);
+  const endMilliseconds = parseInt(moment(start).add(step).format('x'), 10);
+  return endMilliseconds - startMillliseconds;
+}
+
 export const getPlaySliderParams = function (timestamps, timeGrain) {
-  let start = Math.min(...timestamps);
-  let end = Math.max(...timestamps);
+  const minTimestamp = moment(Math.min(...timestamps));
+  const maxTimestamp = moment(Math.max(...timestamps));
   let step;
+  let reference;
 
-  if (timeGrain.indexOf('/') > 0) {
+  if (timeGrain.indexOf('/') !== -1) {
     // Here, time grain is a time interval instead of a simple duration, either
     // `reference/duration` or `duration/reference`. We need to parse the
     // duration and make sure that start and end are in the right places. For
     // example, if `reference` is a Saturday and `duration` is 1 week (P1W)
     // then both start and end should be Saturdays.
     const parts = timeGrain.split('/', 2);
-    let reference;
     if (parts[0].endsWith('Z')) {  // ISO string
-      reference = new Date(parts[0]).getTime();
-      step = parseIsoDuration(parts[1]);
+      reference = moment(parts[0]);
+      step = moment.duration(parts[1]);
     } else {
-      reference = new Date(parts[1]).getTime();
-      step = parseIsoDuration(parts[0]);
+      reference = moment(parts[1]);
+      step = moment.duration(parts[0]);
     }
-    start = reference + step * Math.floor((start - reference) / step);
-    end = reference + step * (Math.floor((end - reference) / step) + 1);
   } else {
-    // lock start and end to the closest steps
-    step = parseIsoDuration(timeGrain);
-    start -= start % step;
-    end += step - end % step;
+    step = moment.duration(timeGrain);
+    reference = truncate(minTimestamp, step);
   }
 
-  const values = timeGrain != null ? [start, start + step] : [start, end];
+  // find the largest `reference + n * step` smaller than the minimum timestamp
+  const start = moment(reference);
+  while (start < minTimestamp) {
+    start.add(step);
+  }
+  while (start > minTimestamp) {
+    start.subtract(step);
+  }
+
+  // find the smallest `reference + n * step` larger than the maximum timestamp
+  const end = moment(reference);
+  while (end > maxTimestamp) {
+    end.subtract(step);
+  }
+  while (end < maxTimestamp) {
+    end.add(step);
+  }
+
+  const values = timeGrain != null ? [start, moment(start).add(step)] : [start, end];
   const disabled = timestamps.every(timestamp => timestamp === null);
 
-  return { start, end, step, values, disabled };
+  return {
+    start: parseInt(start.format('x'), 10),
+    end: parseInt(end.format('x'), 10),
+    getStep: getStepSeconds.bind(this, step),
+    values: values.map(v => parseInt(v.format('x'), 10)),
+    disabled,
+  };
 };
-
diff --git a/superset/assets/src/visualizations/deckgl/AnimatableDeckGLContainer.jsx b/superset/assets/src/visualizations/deckgl/AnimatableDeckGLContainer.jsx
index ddf5a40..cebcee4 100644
--- a/superset/assets/src/visualizations/deckgl/AnimatableDeckGLContainer.jsx
+++ b/superset/assets/src/visualizations/deckgl/AnimatableDeckGLContainer.jsx
@@ -8,7 +8,7 @@ const propTypes = {
   getLayers: PropTypes.func.isRequired,
   start: PropTypes.number.isRequired,
   end: PropTypes.number.isRequired,
-  step: PropTypes.number.isRequired,
+  getStep: PropTypes.func,
   values: PropTypes.array.isRequired,
   aggregation: PropTypes.bool,
   disabled: PropTypes.bool,
@@ -19,13 +19,12 @@ const propTypes = {
 const defaultProps = {
   aggregation: false,
   disabled: false,
-  step: 1,
 };
 
 export default class AnimatableDeckGLContainer extends React.Component {
   constructor(props) {
     super(props);
-    const { getLayers, start, end, step, values, disabled, viewport, ...other } = props;
+    const { getLayers, start, end, getStep, values, disabled, viewport, ...other } = props;
     this.state = { values, viewport };
     this.other = other;
     this.onChange = this.onChange.bind(this);
@@ -37,11 +36,11 @@ export default class AnimatableDeckGLContainer extends React.Component {
     this.setState({
       values: Array.isArray(newValues)
         ? newValues
-        : [newValues, newValues + this.props.step],
+        : [newValues, this.props.getStep(newValues)],
     });
   }
   render() {
-    const { start, end, step, disabled, aggregation, children, getLayers } = this.props;
+    const { start, end, getStep, disabled, aggregation, children, getLayers } = this.props;
     const { values, viewport } = this.state;
     const layers = getLayers(values);
     return (
@@ -56,7 +55,7 @@ export default class AnimatableDeckGLContainer extends React.Component {
         <PlaySlider
           start={start}
           end={end}
-          step={step}
+          step={getStep(start)}
           values={values}
           range={!aggregation}
           onChange={this.onChange}
diff --git a/superset/assets/src/visualizations/deckgl/CategoricalDeckGLContainer.jsx b/superset/assets/src/visualizations/deckgl/CategoricalDeckGLContainer.jsx
index 8145134..ff5ab09 100644
--- a/superset/assets/src/visualizations/deckgl/CategoricalDeckGLContainer.jsx
+++ b/superset/assets/src/visualizations/deckgl/CategoricalDeckGLContainer.jsx
@@ -51,10 +51,10 @@ export default class CategoricalDeckGLContainer extends React.PureComponent {
 
     const timeGrain = fd.time_grain_sqla || fd.granularity || 'PT1M';
     const timestamps = nextProps.payload.data.features.map(f => f.__timestamp);
-    const { start, end, step, values, disabled } = getPlaySliderParams(timestamps, timeGrain);
+    const { start, end, getStep, values, disabled } = getPlaySliderParams(timestamps, timeGrain);
     const categories = getCategories(fd, nextProps.payload.data.features);
 
-    return { start, end, step, values, disabled, categories };
+    return { start, end, getStep, values, disabled, categories };
   }
   constructor(props) {
     super(props);
@@ -134,7 +134,7 @@ export default class CategoricalDeckGLContainer extends React.PureComponent {
           getLayers={this.getLayers}
           start={this.state.start}
           end={this.state.end}
-          step={this.state.step}
+          getStep={this.state.getStep}
           values={this.state.values}
           disabled={this.state.disabled}
           viewport={this.props.viewport}
diff --git a/superset/assets/src/visualizations/deckgl/layers/screengrid.jsx b/superset/assets/src/visualizations/deckgl/layers/screengrid.jsx
index 4ec6e8d..b4df577 100644
--- a/superset/assets/src/visualizations/deckgl/layers/screengrid.jsx
+++ b/superset/assets/src/visualizations/deckgl/layers/screengrid.jsx
@@ -65,9 +65,9 @@ class DeckGLScreenGrid extends React.PureComponent {
 
     const timeGrain = fd.time_grain_sqla || fd.granularity || 'PT1M';
     const timestamps = nextProps.payload.data.features.map(f => f.__timestamp);
-    const { start, end, step, values, disabled } = getPlaySliderParams(timestamps, timeGrain);
+    const { start, end, getStep, values, disabled } = getPlaySliderParams(timestamps, timeGrain);
 
-    return { start, end, step, values, disabled };
+    return { start, end, getStep, values, disabled };
   }
   constructor(props) {
     super(props);
@@ -103,7 +103,7 @@ class DeckGLScreenGrid extends React.PureComponent {
           getLayers={this.getLayers}
           start={this.state.start}
           end={this.state.end}
-          step={this.state.step}
+          getStep={this.state.getStep}
           values={this.state.values}
           disabled={this.state.disabled}
           viewport={this.props.viewport}
diff --git a/superset/assets/yarn.lock b/superset/assets/yarn.lock
index 51d6b14..5f38f7d 100644
--- a/superset/assets/yarn.lock
+++ b/superset/assets/yarn.lock
@@ -8987,10 +8987,6 @@ parse-headers@^2.0.0:
     for-each "^0.3.2"
     trim "0.0.1"
 
-parse-iso-duration@^1.0.0:
-  version "1.0.0"
-  resolved "https://registry.yarnpkg.com/parse-iso-duration/-/parse-iso-duration-1.0.0.tgz#b923ab898a8ff8f42bdc9ee5db6e22808c48a864"
-
 parse-json@^2.2.0:
   version "2.2.0"
   resolved "https://registry.yarnpkg.com/parse-json/-/parse-json-2.2.0.tgz#f480f40434ef80741f8469099f8dea18f55a4dc9"