You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2018/09/10 17:28:19 UTC
[GitHub] mistercrunch closed pull request #5785: Handle "ambiguous durations"
mistercrunch closed pull request #5785: Handle "ambiguous durations"
URL: https://github.com/apache/incubator-superset/pull/5785
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/superset/assets/package.json b/superset/assets/package.json
index f36ffdeb5d..b6080593b5 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 59dab8effe..36260a461f 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 7ebc4d7766..8b9437fe86 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 ddf5a4028b..cebcee4364 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 81451345f8..ff5ab09381 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 4ec6e8d77b..b4df577d78 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 2b330defcf..c410ef4e39 100644
--- a/superset/assets/yarn.lock
+++ b/superset/assets/yarn.lock
@@ -1061,7 +1061,7 @@ ajv@^4.9.1:
co "^4.6.0"
json-stable-stringify "^1.0.1"
-ajv@^5.2.3, ajv@^5.3.0:
+ajv@^5.1.0, ajv@^5.2.3, ajv@^5.3.0:
version "5.5.2"
resolved "https://registry.yarnpkg.com/ajv/-/ajv-5.5.2.tgz#73b5eeca3fab653e3d3f9422b341ad42205dc965"
dependencies:
@@ -1418,7 +1418,7 @@ aws-sign@~0.3.0:
version "0.3.0"
resolved "https://registry.yarnpkg.com/aws-sign/-/aws-sign-0.3.0.tgz#3d81ca69b474b1e16518728b51c24ff0bbedc6e9"
-aws4@^1.2.1, aws4@^1.8.0:
+aws4@^1.2.1, aws4@^1.6.0, aws4@^1.8.0:
version "1.8.0"
resolved "https://registry.yarnpkg.com/aws4/-/aws4-1.8.0.tgz#f0e003d9ca9e7f59c7a508945d7b2ef9a04a542f"
@@ -4872,7 +4872,7 @@ extend-shallow@^3.0.0, extend-shallow@^3.0.2:
assign-symbols "^1.0.0"
is-extendable "^1.0.1"
-extend@^3.0.0, extend@~3.0.0, extend@~3.0.2:
+extend@^3.0.0, extend@~3.0.0, extend@~3.0.1, extend@~3.0.2:
version "3.0.2"
resolved "https://registry.yarnpkg.com/extend/-/extend-3.0.2.tgz#f8b1136b4071fbd8eb140aff858b1019ec2915fa"
@@ -5210,7 +5210,7 @@ form-data@~2.1.1:
combined-stream "^1.0.5"
mime-types "^2.1.12"
-form-data@~2.3.2:
+form-data@~2.3.1, form-data@~2.3.2:
version "2.3.2"
resolved "https://registry.yarnpkg.com/form-data/-/form-data-2.3.2.tgz#4970498be604c20c005d4f5c23aecd21d6b49099"
dependencies:
@@ -5837,6 +5837,13 @@ har-validator@~4.2.1:
ajv "^4.9.1"
har-schema "^1.0.5"
+har-validator@~5.0.3:
+ version "5.0.3"
+ resolved "https://registry.yarnpkg.com/har-validator/-/har-validator-5.0.3.tgz#ba402c266194f15956ef15e0fcf242993f6a7dfd"
+ dependencies:
+ ajv "^5.1.0"
+ har-schema "^2.0.0"
+
har-validator@~5.1.0:
version "5.1.0"
resolved "https://registry.yarnpkg.com/har-validator/-/har-validator-5.1.0.tgz#44657f5688a22cfd4b72486e81b3a3fb11742c29"
@@ -8617,7 +8624,7 @@ oauth-sign@~0.3.0:
version "0.3.0"
resolved "https://registry.yarnpkg.com/oauth-sign/-/oauth-sign-0.3.0.tgz#cb540f93bb2b22a7d5941691a288d60e8ea9386e"
-oauth-sign@~0.8.1:
+oauth-sign@~0.8.1, oauth-sign@~0.8.2:
version "0.8.2"
resolved "https://registry.yarnpkg.com/oauth-sign/-/oauth-sign-0.8.2.tgz#46a6ab7f0aead8deae9ec0565780b7d4efeb9d43"
@@ -8986,10 +8993,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"
@@ -9864,7 +9867,7 @@ qs@~6.4.0:
version "6.4.0"
resolved "https://registry.yarnpkg.com/qs/-/qs-6.4.0.tgz#13e26d28ad6b0ffaa91312cd3bf708ed351e7233"
-qs@~6.5.2:
+qs@~6.5.1, qs@~6.5.2:
version "6.5.2"
resolved "https://registry.yarnpkg.com/qs/-/qs-6.5.2.tgz#cb3ae806e8740444584ef154ce8ee98d403f3e36"
@@ -12030,7 +12033,7 @@ tough-cookie@^2.3.2, tough-cookie@~2.4.3:
psl "^1.1.24"
punycode "^1.4.1"
-tough-cookie@~2.3.0:
+tough-cookie@~2.3.0, tough-cookie@~2.3.3:
version "2.3.4"
resolved "https://registry.yarnpkg.com/tough-cookie/-/tough-cookie-2.3.4.tgz#ec60cee38ac675063ffc97a5c18970578ee83655"
dependencies:
@@ -12408,7 +12411,7 @@ utils-merge@1.0.1:
version "1.0.1"
resolved "https://registry.yarnpkg.com/utils-merge/-/utils-merge-1.0.1.tgz#9f95710f50a267947b2ccc124741c1028427e713"
-uuid@^3.0.0, uuid@^3.0.1, uuid@^3.3.2:
+uuid@^3.0.0, uuid@^3.0.1, uuid@^3.1.0, uuid@^3.3.2:
version "3.3.2"
resolved "https://registry.yarnpkg.com/uuid/-/uuid-3.3.2.tgz#1b4af4955eb3077c501c23872fc6513811587131"
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org