Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing time comparison to look for past deltas #7616

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions UPDATING.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ creation of permissions set `FAB_UPDATE_PERMS = False` on config.
which adds missing non-nullable fields and uniqueness constraints to the metrics
and sql_metrics tables. Depending on the integrity of the data, manual
intervention may be required.
* [7616](https://github.com/apache/incubator-superset/pull/7616): this bug fix
changes time_compare deltas to correctly evaluate to the number of days prior
instead of number of days in the future. It will change the data for advanced
analytics time_compare so `1 year` from 5/1/2019 will be calculated as 365 days
instead of 366 days.

## Superset 0.32.0

Expand Down
21 changes: 17 additions & 4 deletions superset/utils/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ def default(self, o):
return json.JSONEncoder.default(self, o)


def parse_human_timedelta(s: str):
def parse_human_timedelta(s: str) -> timedelta:
"""
Returns ``datetime.datetime`` from natural language time deltas

Expand All @@ -312,6 +312,19 @@ def parse_human_timedelta(s: str):
return d - dttm


def parse_past_timedelta(delta_str: str) -> timedelta:
"""
Takes a delta like '1 year' and finds the timedelta for that period in
the past, then represents that past timedelta in positive terms.

parse_human_timedelta('1 year') find the timedelta 1 year in the future.
parse_past_timedelta('1 year') returns -datetime.timedelta(-365)
or datetime.timedelta(365).
"""
return -parse_human_timedelta(
delta_str if delta_str.startswith('-') else f'-{delta_str}')


class JSONEncodedDict(TypeDecorator):
"""Represents an immutable structure as a json-encoded string."""

Expand Down Expand Up @@ -1003,9 +1016,9 @@ def get_since_until(time_range: Optional[str] = None,
until = parse_human_datetime(until) if until else relative_end

if time_shift:
time_shift = parse_human_timedelta(time_shift)
since = since if since is None else (since - time_shift) # noqa: T400
until = until if until is None else (until - time_shift) # noqa: T400
time_delta = parse_past_timedelta(time_shift)
since = since if since is None else (since - time_delta) # noqa: T400
until = until if until is None else (until - time_delta) # noqa: T400

if since and until and since > until:
raise ValueError(_('From date cannot be larger than to date'))
Expand Down
4 changes: 2 additions & 2 deletions superset/viz.py
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ def query_obj(self):
since=form_data.get('since'),
until=form_data.get('until'))
time_shift = form_data.get('time_shift', '')
self.time_shift = utils.parse_human_timedelta(time_shift)
self.time_shift = utils.parse_past_timedelta(time_shift)
from_dttm = None if since is None else (since - self.time_shift)
to_dttm = None if until is None else (until - self.time_shift)
if from_dttm and to_dttm and from_dttm > to_dttm:
Expand Down Expand Up @@ -1210,7 +1210,7 @@ def run_extra_queries(self):

for option in time_compare:
query_object = self.query_obj()
delta = utils.parse_human_timedelta(option)
delta = utils.parse_past_timedelta(option)
query_object['inner_from_dttm'] = query_object['from_dttm']
query_object['inner_to_dttm'] = query_object['to_dttm']

Expand Down
17 changes: 15 additions & 2 deletions tests/utils_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
merge_request_params,
parse_human_timedelta,
parse_js_uri_path_item,
parse_past_timedelta,
validate_json,
zlib_compress,
zlib_decompress_to_string,
Expand Down Expand Up @@ -119,9 +120,21 @@ def test_base_json_conv(self):
assert isinstance(base_json_conv(uuid.uuid4()), str) is True

@patch('superset.utils.core.datetime')
def test_parse_human_timedelta(self, mock_now):
mock_now.return_value = datetime(2016, 12, 1)
def test_parse_human_timedelta(self, mock_datetime):
mock_datetime.now.return_value = datetime(2019, 4, 1)
mock_datetime.side_effect = lambda *args, **kw: datetime(*args, **kw)
self.assertEquals(parse_human_timedelta('now'), timedelta(0))
self.assertEquals(parse_human_timedelta('1 year'), timedelta(366))
self.assertEquals(parse_human_timedelta('-1 year'), timedelta(-365))

@patch('superset.utils.core.datetime')
def test_parse_past_timedelta(self, mock_datetime):
mock_datetime.now.return_value = datetime(2019, 4, 1)
mock_datetime.side_effect = lambda *args, **kw: datetime(*args, **kw)
self.assertEquals(parse_past_timedelta('1 year'), timedelta(365))
self.assertEquals(parse_past_timedelta('-1 year'), timedelta(365))
self.assertEquals(parse_past_timedelta('52 weeks'), timedelta(364))
self.assertEquals(parse_past_timedelta('1 month'), timedelta(31))

def test_zlib_compression(self):
json_str = '{"test": 1}'
Expand Down