You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by jl...@apache.org on 2026/07/14 22:22:25 UTC
(superset) 01/01: fix(charts): render time comparison without a time grain
This is an automated email from the ASF dual-hosted git repository.
sadpandajoe pushed a commit to branch claude/superset-time-comparison-grain-dd1acc
in repository https://gitbox.apache.org/repos/asf/superset.git
commit daae66f4c44429de558bdd28ba0ca2e5273fb991
Author: Joe Li <jo...@preset.io>
AuthorDate: Tue Jul 14 15:22:11 2026 -0700
fix(charts): render time comparison without a time grain
A time-series chart using a relative time-comparison offset with no time
grain returned a 400 ("Time Grain must be specified when using Time
Comparison") instead of rendering. The validation was introduced as a
side effect of the time-shift join-column rework; before that, grain-less
comparisons joined current against historic timestamps and silently
produced an empty comparison series.
Instead of raising (or restoring the silently-broken raw join), align
grain-less relative offsets exactly: shift the main series' timestamps by
the offset delta and join them against the offset series' raw timestamps,
mirroring the grain-based join without truncation. Joining on stringified
values keeps the merge safe when an empty offset series materializes its
join keys as NaN floats.
Also, as part of this path:
- normalize_time_delta converts quarters to months, since pd.DateOffset
rejects a `quarters` argument (this previously crashed grain-based
quarter offsets too)
- free-form offsets (e.g. "one year ago") that fail the digit-only
grammar are shifted with the same parser used for the offset query's
time range, with a logged fallback when the offset is uninterpretable
- the query's x-axis label picks the temporal join key instead of the
first datetime column
- grain-less joins preserve the original column order, which the
set_index/join/reset_index round-trip otherwise rearranges
Co-Authored-By: Claude Fable 5 <no...@anthropic.com>
---
superset/models/helpers.py | 118 ++++++++++---
superset/utils/date_parser.py | 3 +
tests/unit_tests/common/test_time_shifts.py | 253 +++++++++++++++++++++++++---
tests/unit_tests/utils/date_parser_tests.py | 14 ++
4 files changed, 344 insertions(+), 44 deletions(-)
diff --git a/superset/models/helpers.py b/superset/models/helpers.py
index d6cd79d537c..90bbac3545f 100644
--- a/superset/models/helpers.py
+++ b/superset/models/helpers.py
@@ -2118,6 +2118,7 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
time_grain,
join_keys,
full_range=getattr(query_object, "time_compare_full_range", False),
+ x_axis_label=get_x_axis_label(query_object.columns),
)
return CachedTimeOffset(df=df, queries=queries, cache_keys=cache_keys)
@@ -2268,7 +2269,7 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
)
else:
df.drop(
- list(df.filter(regex=f"{R_SUFFIX}")),
+ list(df.filter(regex=f"{OFFSET_JOIN_COLUMN_SUFFIX}|{R_SUFFIX}")),
axis=1,
inplace=True,
)
@@ -2284,6 +2285,7 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
join_keys: list[str],
is_date_range_offset: bool,
join_column_producer: Any,
+ x_axis_label: str | None = None,
) -> tuple[pd.DataFrame, list[str]]:
"""Determine appropriate join keys and modify DataFrames if needed."""
if time_grain and not is_date_range_offset:
@@ -2312,8 +2314,87 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
return self._process_date_range_offset(offset_df, join_keys)
else:
+ return self._align_offset_without_time_grain(
+ df, offset_df, offset, join_keys, x_axis_label
+ )
+
+ def _align_offset_without_time_grain(
+ self,
+ df: pd.DataFrame,
+ offset_df: pd.DataFrame,
+ offset: str,
+ join_keys: list[str],
+ x_axis_label: str | None = None,
+ ) -> tuple[pd.DataFrame, list[str]]:
+ """
+ Determine join keys for a relative offset when no time grain is set.
+
+ Without a time grain there is no truncated join column, but the two
+ series can still be aligned exactly: shifting the main series'
+ timestamps by the offset delta lands them on the offset series' raw
+ timestamps (normalize_time_delta returns a negative delta for "... ago"
+ offsets). Timestamps without an exact counterpart in the offset series
+ produce nulls, mirroring the grain-based join, and null timestamps in
+ the two series join to each other (both stringify to "NaT"). When
+ there is no temporal join key, or the offset cannot be interpreted as
+ a time delta, the original join keys are used as-is.
+ """
+ # Prefer the query's temporal x-axis when it is a join key; otherwise
+ # use the first datetime join key.
+ candidate_keys = sorted(join_keys, key=lambda key: key != x_axis_label)
+ temporal_join_key = next(
+ (
+ key
+ for key in candidate_keys
+ if key in df.columns
+ and key in offset_df.columns
+ and pd.api.types.is_datetime64_any_dtype(df[key])
+ ),
+ None,
+ )
+ if not temporal_join_key:
return offset_df, join_keys
+ try:
+ delta: DateOffset | None = DateOffset(**normalize_time_delta(offset))
+ except (ValueError, TimeDeltaAmbiguousError):
+ delta = None
+
+ column_name = OFFSET_JOIN_COLUMN_SUFFIX + offset
+ if delta is not None:
+ shifted = df[temporal_join_key].map(lambda value: value + delta)
+ else:
+ # Free-form offsets (e.g. "one year ago") don't match the
+ # normalize_time_delta grammar; shift with the same parser that
+ # shifted the offset query's time range. parsedatetime works at
+ # second resolution, so sub-second or timezone-aware timestamps
+ # won't align on this path.
+ probe = df[temporal_join_key].dropna()
+ if (
+ not probe.empty
+ and get_past_or_future(offset, probe.iloc[0]) == (probe.iloc[0])
+ ):
+ logger.warning(
+ "Cannot interpret time offset %r without a time grain; "
+ "the time comparison may be incorrect.",
+ offset,
+ )
+ return offset_df, join_keys
+ shifted = df[temporal_join_key].map(
+ lambda value: value
+ if pd.isna(value)
+ else get_past_or_future(offset, value)
+ )
+
+ # Join on string values so that mismatched key dtypes (e.g. an empty
+ # offset series materializes its join keys as NaN floats) cannot break
+ # the merge.
+ df[column_name] = shifted.map(str)
+ offset_df[column_name] = offset_df[temporal_join_key].map(str)
+
+ remaining_keys = [key for key in join_keys if key != temporal_join_key]
+ return offset_df, [column_name, *remaining_keys]
+
def _perform_join(
self,
df: pd.DataFrame,
@@ -2357,6 +2438,7 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
time_grain: str | None,
join_keys: list[str],
full_range: bool = False,
+ x_axis_label: str | None = None,
) -> pd.DataFrame:
"""
Join offset DataFrames with the main DataFrame.
@@ -2369,31 +2451,13 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
time range instead of being truncated to the main series' range. This
uses an outer join so offset-only rows (e.g. the rest of a prior day when
the current day is still in progress) are preserved.
+ :param x_axis_label: The query's temporal x-axis label, used to pick the
+ temporal join key when no time grain is set.
"""
join_column_producer = app.config["TIME_GRAIN_JOIN_COLUMN_PRODUCERS"].get(
time_grain
)
-
- if not time_grain:
- has_temporal_join_key = any(
- pd.api.types.is_datetime64_any_dtype(df[key])
- for key in join_keys
- if key in df.columns
- )
- if has_temporal_join_key:
- has_relative_offset = any(
- not (
- self.is_valid_date_range(offset)
- and feature_flag_manager.is_feature_enabled(
- "DATE_RANGE_TIMESHIFTS_ENABLED"
- )
- )
- for offset in offset_dfs
- )
- if has_relative_offset:
- raise QueryObjectValidationError(
- _("Time Grain must be specified when using Time Comparison.")
- )
+ original_columns = list(df.columns)
for offset, offset_df in offset_dfs.items():
is_date_range_offset = self.is_valid_date_range(
@@ -2410,6 +2474,7 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
join_keys,
is_date_range_offset,
join_column_producer,
+ x_axis_label,
)
# The full-range option is only meaningful for relative offsets aligned
@@ -2432,6 +2497,15 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
df, offset, time_grain, join_keys, is_date_range_offset
)
+ if not time_grain and not is_date_range_offset:
+ # The grain-less join indexes on the synthetic key plus the
+ # non-temporal join keys, which reset_index moves to the front;
+ # restore the original column order.
+ df = df[
+ [col for col in original_columns if col in df.columns]
+ + [col for col in df.columns if col not in original_columns]
+ ]
+
return df
def _coalesce_offset_index(
diff --git a/superset/utils/date_parser.py b/superset/utils/date_parser.py
index 7405cba27a5..3281db1c2cb 100644
--- a/superset/utils/date_parser.py
+++ b/superset/utils/date_parser.py
@@ -98,6 +98,9 @@ def normalize_time_delta(human_readable: str) -> dict[str, int]:
key = matched[2] + "s"
value = int(matched[1])
value = -value if matched[3] == "ago" else value
+ if key == "quarters":
+ # pd.DateOffset does not accept a `quarters` argument
+ key, value = "months", value * 3
return {key: value}
diff --git a/tests/unit_tests/common/test_time_shifts.py b/tests/unit_tests/common/test_time_shifts.py
index a34076393f2..56975f56a25 100644
--- a/tests/unit_tests/common/test_time_shifts.py
+++ b/tests/unit_tests/common/test_time_shifts.py
@@ -14,7 +14,6 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-import pytest
from pandas import DataFrame, Series, Timestamp
from pandas.testing import assert_frame_equal
from pytest import fixture, mark # noqa: PT013
@@ -24,7 +23,6 @@ from superset.common.query_context import QueryContext
from superset.common.query_context_processor import QueryContextProcessor
from superset.connectors.sqla.models import BaseDatasource
from superset.constants import TimeGrain
-from superset.exceptions import QueryObjectValidationError
from superset.models.helpers import ExploreMixin
# Create processor and bind ExploreMixin methods to datasource
@@ -51,6 +49,9 @@ _datasource.is_valid_date_range = ExploreMixin.is_valid_date_range.__get__(_data
_datasource._determine_join_keys = ExploreMixin._determine_join_keys.__get__(
_datasource
)
+_datasource._align_offset_without_time_grain = (
+ ExploreMixin._align_offset_without_time_grain.__get__(_datasource)
+)
_datasource._perform_join = ExploreMixin._perform_join.__get__(_datasource)
_datasource._apply_cleanup_logic = ExploreMixin._apply_cleanup_logic.__get__(
_datasource
@@ -334,18 +335,215 @@ def test_join_offset_dfs_totals_query_no_dimensions():
assert_frame_equal(expected, result)
-def test_join_offset_dfs_raises_without_time_grain():
- """Time comparison with relative offsets requires a time grain."""
+def test_join_offset_dfs_no_time_grain_aligns_relative_offset():
+ """
+ Without a time grain, a relative offset joins on the exact shifted
+ timestamps instead of raising, so saved charts without a grain render
+ with a correctly aligned comparison series.
+ """
+ df = DataFrame(
+ {
+ "ds": [Timestamp("2021-01-01"), Timestamp("2021-02-01")],
+ "D": [1, 2],
+ }
+ )
+ offset_df = DataFrame(
+ {
+ "ds": [Timestamp("2020-01-01"), Timestamp("2020-02-01")],
+ "B": [5, 6],
+ }
+ )
+ offset_dfs = {"1 year ago": offset_df}
+
+ expected = DataFrame(
+ {
+ "ds": [Timestamp("2021-01-01"), Timestamp("2021-02-01")],
+ "D": [1, 2],
+ "B": [5, 6],
+ }
+ )
+
+ result = query_context_processor.join_offset_dfs(
+ df, offset_dfs, time_grain=None, join_keys=["ds"]
+ )
+
+ assert_frame_equal(expected, result)
+
+
+def test_join_offset_dfs_no_time_grain_unmatched_timestamps_yield_nulls():
+ """
+ Without a time grain, offset timestamps that have no exact shifted
+ counterpart in the main series produce nulls instead of raising.
+ """
+ df = DataFrame({"ds": [Timestamp("2021-01-01")], "D": [1]})
+ offset_df = DataFrame({"ds": [Timestamp("2020-06-15")], "B": [5]})
+ offset_dfs = {"1 year ago": offset_df}
+
+ result = query_context_processor.join_offset_dfs(
+ df, offset_dfs, time_grain=None, join_keys=["ds"]
+ )
+
+ assert "B" in result.columns
+ assert result["B"].isna().all()
+
+
+def test_join_offset_dfs_no_time_grain_multiple_offsets():
+ """
+ Multiple relative offsets without a time grain each align on their own
+ shifted timestamps, and no synthetic join columns leak into the result.
+ """
+ df = DataFrame({"ds": [Timestamp("2021-01-29")], "D": [1]})
+ offset_df1 = DataFrame({"ds": [Timestamp("2021-01-01")], "B": [5]})
+ offset_df2 = DataFrame({"ds": [Timestamp("2020-01-29")], "C": [7]})
+ offset_dfs = {"28 days ago": offset_df1, "1 year ago": offset_df2}
+
+ expected = DataFrame(
+ {
+ "ds": [Timestamp("2021-01-29")],
+ "D": [1],
+ "B": [5],
+ "C": [7],
+ }
+ )
+
+ result = query_context_processor.join_offset_dfs(
+ df, offset_dfs, time_grain=None, join_keys=["ds"]
+ )
+
+ assert_frame_equal(expected, result)
+
+
+def test_join_offset_dfs_no_time_grain_empty_offset_df():
+ """
+ An empty offset series materializes its join keys as NaN floats; the
+ grain-less join must not crash on the dtype mismatch.
+ """
df = DataFrame({"ds": [Timestamp("2021-01-01")], "D": [1]})
- offset_df = DataFrame({"ds": [Timestamp("2021-02-01")], "B": [5]})
+ offset_df = DataFrame({"ds": [float("nan")], "B": [float("nan")]})
offset_dfs = {"1 year ago": offset_df}
- with pytest.raises(
- QueryObjectValidationError, match="Time Grain must be specified"
- ):
- query_context_processor.join_offset_dfs(
- df, offset_dfs, time_grain=None, join_keys=["ds"]
- )
+ result = query_context_processor.join_offset_dfs(
+ df, offset_dfs, time_grain=None, join_keys=["ds"]
+ )
+
+ assert "B" in result.columns
+ assert result["B"].isna().all()
+
+
+def test_join_offset_dfs_no_time_grain_quarter_offset():
+ """
+ Quarter offsets align without a time grain (normalize_time_delta converts
+ quarters to months, since pd.DateOffset has no quarters argument).
+ """
+ df = DataFrame({"ds": [Timestamp("2021-04-01")], "D": [1]})
+ offset_df = DataFrame({"ds": [Timestamp("2021-01-01")], "B": [5]})
+ offset_dfs = {"1 quarter ago": offset_df}
+
+ expected = DataFrame({"ds": [Timestamp("2021-04-01")], "D": [1], "B": [5]})
+
+ result = query_context_processor.join_offset_dfs(
+ df, offset_dfs, time_grain=None, join_keys=["ds"]
+ )
+
+ assert_frame_equal(expected, result)
+
+
+def test_join_offset_dfs_no_time_grain_free_form_offset():
+ """
+ Offsets outside the normalize_time_delta grammar (e.g. "one year ago")
+ are aligned with the same parser that shifted the offset query's range.
+ """
+ df = DataFrame({"ds": [Timestamp("2021-01-01")], "D": [1]})
+ offset_df = DataFrame({"ds": [Timestamp("2020-01-01")], "B": [5]})
+ offset_dfs = {"one year ago": offset_df}
+
+ expected = DataFrame({"ds": [Timestamp("2021-01-01")], "D": [1], "B": [5]})
+
+ result = query_context_processor.join_offset_dfs(
+ df, offset_dfs, time_grain=None, join_keys=["ds"]
+ )
+
+ assert_frame_equal(expected, result)
+
+
+def test_join_offset_dfs_no_time_grain_uninterpretable_offset():
+ """
+ An offset that no parser can interpret falls back to joining on the raw
+ keys (an empty comparison series) instead of crashing.
+ """
+ df = DataFrame({"ds": [Timestamp("2021-01-01")], "D": [1]})
+ offset_df = DataFrame({"ds": [Timestamp("2020-01-01")], "B": [5]})
+ offset_dfs = {"not a real offset": offset_df}
+
+ result = query_context_processor.join_offset_dfs(
+ df, offset_dfs, time_grain=None, join_keys=["ds"]
+ )
+
+ assert "B" in result.columns
+ assert result["B"].isna().all()
+
+
+def test_join_offset_dfs_no_time_grain_prefers_x_axis_label():
+ """
+ With multiple datetime join keys, the query's x-axis is the one shifted
+ for alignment, not whichever datetime column happens to come first.
+ """
+ df = DataFrame(
+ {
+ "birth_date": [Timestamp("1990-05-05")],
+ "ds": [Timestamp("2021-02-01")],
+ "D": [1],
+ }
+ )
+ offset_df = DataFrame(
+ {
+ "birth_date": [Timestamp("1990-05-05")],
+ "ds": [Timestamp("2021-01-01")],
+ "B": [5],
+ }
+ )
+ offset_dfs = {"1 month ago": offset_df}
+
+ result = query_context_processor.join_offset_dfs(
+ df,
+ offset_dfs,
+ time_grain=None,
+ join_keys=["birth_date", "ds"],
+ x_axis_label="ds",
+ )
+
+ assert result["B"].tolist() == [5]
+
+
+def test_join_offset_dfs_no_time_grain_preserves_column_order():
+ """
+ A join key following the temporal key must not change the result's
+ column order (chart payloads and CSV exports derive order from columns).
+ """
+ df = DataFrame(
+ {
+ "country": ["US"],
+ "ds": [Timestamp("2021-02-01")],
+ "category": ["a"],
+ "D": [1],
+ }
+ )
+ offset_df = DataFrame(
+ {
+ "country": ["US"],
+ "ds": [Timestamp("2021-01-01")],
+ "category": ["a"],
+ "B": [5],
+ }
+ )
+ offset_dfs = {"1 month ago": offset_df}
+
+ result = query_context_processor.join_offset_dfs(
+ df, offset_dfs, time_grain=None, join_keys=["country", "ds", "category"]
+ )
+
+ assert result.columns.tolist() == ["country", "ds", "category", "D", "B"]
+ assert result["B"].tolist() == [5]
def test_join_offset_dfs_allows_non_temporal_join_without_time_grain():
@@ -360,27 +558,38 @@ def test_join_offset_dfs_allows_non_temporal_join_without_time_grain():
assert "metric__1 year ago" in result.columns
-def test_join_offset_dfs_raises_when_temporal_key_not_first():
- """Temporal join key detection works even when it's not the first key."""
+def test_join_offset_dfs_no_time_grain_temporal_key_not_first():
+ """
+ The temporal join key is aligned by shifting even when it is not the
+ first join key; remaining keys still participate in the join.
+ """
df = DataFrame(
{
"country": ["US", "UK"],
- "ds": [Timestamp("2021-01-01"), Timestamp("2021-02-01")],
+ "ds": [Timestamp("2021-02-01"), Timestamp("2021-02-01")],
"D": [1, 2],
}
)
offset_df = DataFrame(
{
"country": ["US", "UK"],
- "ds": [Timestamp("2021-03-01"), Timestamp("2021-04-01")],
+ "ds": [Timestamp("2021-01-01"), Timestamp("2021-01-01")],
"B": [5, 6],
}
)
- offset_dfs = {"1 year ago": offset_df}
+ offset_dfs = {"1 month ago": offset_df}
- with pytest.raises(
- QueryObjectValidationError, match="Time Grain must be specified"
- ):
- query_context_processor.join_offset_dfs(
- df, offset_dfs, time_grain=None, join_keys=["country", "ds"]
- )
+ expected = DataFrame(
+ {
+ "country": ["US", "UK"],
+ "ds": [Timestamp("2021-02-01"), Timestamp("2021-02-01")],
+ "D": [1, 2],
+ "B": [5, 6],
+ }
+ )
+
+ result = query_context_processor.join_offset_dfs(
+ df, offset_dfs, time_grain=None, join_keys=["country", "ds"]
+ )
+
+ assert_frame_equal(expected, result)
diff --git a/tests/unit_tests/utils/date_parser_tests.py b/tests/unit_tests/utils/date_parser_tests.py
index 5d275f2f271..b30d22a9ef0 100644
--- a/tests/unit_tests/utils/date_parser_tests.py
+++ b/tests/unit_tests/utils/date_parser_tests.py
@@ -32,9 +32,11 @@ from superset.utils.date_parser import (
datetime_eval,
get_past_or_future,
get_since_until,
+ normalize_time_delta,
parse_human_datetime,
parse_human_timedelta,
parse_past_timedelta,
+ TimeDeltaAmbiguousError,
)
from tests.unit_tests.conftest import with_feature_flags
@@ -561,6 +563,18 @@ def test_get_past_or_future() -> None:
assert get_past_or_future("3 month", dttm) == datetime(2020, 5, 29)
+def test_normalize_time_delta() -> None:
+ assert normalize_time_delta("1 year ago") == {"years": -1}
+ assert normalize_time_delta("2 months later") == {"months": 2}
+ assert normalize_time_delta("28 days ago") == {"days": -28}
+ # quarters are converted to months (pd.DateOffset has no quarters argument)
+ assert normalize_time_delta("1 quarter ago") == {"months": -3}
+ assert normalize_time_delta("2 quarters later") == {"months": 6}
+
+ with pytest.raises(TimeDeltaAmbiguousError):
+ normalize_time_delta("one year ago")
+
+
def test_parse_human_datetime() -> None:
with pytest.raises(TimeRangeAmbiguousError):
parse_human_datetime("2 days")