sadpandajoe commented on code in PR #42054:
URL: https://github.com/apache/superset/pull/42054#discussion_r3590974550
##########
tests/unit_tests/common/test_time_shifts.py:
##########
@@ -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():
Review Comment:
Added `-> None` return annotations in aebbacc2a9 — to all 10 new test
functions in this file, not just the flagged ones.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
tests/unit_tests/common/test_time_shifts.py:
##########
@@ -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():
Review Comment:
Added `-> None` return annotations in aebbacc2a9 — to all 10 new test
functions in this file, not just the flagged ones.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
tests/unit_tests/common/test_time_shifts.py:
##########
@@ -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():
Review Comment:
Added `-> None` return annotations in aebbacc2a9 — to all 10 new test
functions in this file, not just the flagged ones.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
tests/unit_tests/common/test_time_shifts.py:
##########
@@ -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():
Review Comment:
Added `-> None` return annotations in aebbacc2a9 — to all 10 new test
functions in this file, not just the flagged ones.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
tests/unit_tests/common/test_time_shifts.py:
##########
@@ -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():
Review Comment:
Added `-> None` return annotations in aebbacc2a9 — to all 10 new test
functions in this file, not just the flagged ones.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
