codeant-ai-for-open-source[bot] commented on code in PR #42054:
URL: https://github.com/apache/superset/pull/42054#discussion_r3590956425
##########
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:
**Suggestion:** Add an explicit return type annotation to this new test
function to comply with the required typing standard. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This is a newly added Python test function with no return type annotation,
which violates the type-hint requirement for new or modified Python code.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b095c8f061834f93825e4edb73566e67&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=b095c8f061834f93825e4edb73566e67&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/common/test_time_shifts.py
**Line:** 338:338
**Comment:**
*Custom Rule: Add an explicit return type annotation to this new test
function to comply with the required typing standard.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42054&comment_hash=1483d10a595871e521342238427b09174a962b7c84d34effe101469b52e7641b&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42054&comment_hash=1483d10a595871e521342238427b09174a962b7c84d34effe101469b52e7641b&reaction=dislike'>๐</a>
##########
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:
**Suggestion:** Add an explicit return type annotation to this newly
introduced test function. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
The new test function has no return type annotation, which is a real
violation of the project rule requiring type hints on newly added Python code.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b004f3da51ad4ed385bae194ac5012f0&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=b004f3da51ad4ed385bae194ac5012f0&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/common/test_time_shifts.py
**Line:** 390:390
**Comment:**
*Custom Rule: Add an explicit return type annotation to this newly
introduced test function.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42054&comment_hash=2140ed9c8910a98439194fe3e78beb21c9d839fbccb16425cffa4d97789bc48f&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42054&comment_hash=2140ed9c8910a98439194fe3e78beb21c9d839fbccb16425cffa4d97789bc48f&reaction=dislike'>๐</a>
##########
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:
**Suggestion:** Add an explicit return type annotation to this new test
function to satisfy the mandatory type-hint rule. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This newly introduced test function omits a return type annotation, so it
matches the custom rule requiring type hints on new Python functions.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=95017e6bc80e4a7cbc812758cb79726e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=95017e6bc80e4a7cbc812758cb79726e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/common/test_time_shifts.py
**Line:** 373:373
**Comment:**
*Custom Rule: Add an explicit return type annotation to this new test
function to satisfy the mandatory type-hint rule.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42054&comment_hash=f75987b06fc6fe44a6151e32e84fe690850e2758b66d96d16dbf076611205bec&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42054&comment_hash=f75987b06fc6fe44a6151e32e84fe690850e2758b66d96d16dbf076611205bec&reaction=dislike'>๐</a>
##########
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:
**Suggestion:** Add an explicit return type annotation to this test function
to align with the type-hint requirement. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This newly added test function lacks a return type annotation, so the
type-hint rule applies here.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b8675998182b417fa3567c762b8750f8&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=b8675998182b417fa3567c762b8750f8&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/common/test_time_shifts.py
**Line:** 416:416
**Comment:**
*Custom Rule: Add an explicit return type annotation to this test
function to align with the type-hint requirement.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42054&comment_hash=f75b1a7a29cd512d693b0931ee3c2a1995ffdca41a7cb89033485d3f3b864c1e&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42054&comment_hash=f75b1a7a29cd512d693b0931ee3c2a1995ffdca41a7cb89033485d3f3b864c1e&reaction=dislike'>๐</a>
##########
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:
**Suggestion:** Add an explicit return type annotation to this new test
function to comply with the rule requiring type hints. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This is another new Python test function without any type annotation, which
is a direct match for the required typing standard.
</details>
<details>
<summary><b>Rule source ๐ </b></summary>
.cursor/rules/dev-standard.mdc (line 28)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d73c217821bf407ba1c35b8d0c5a7093&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=d73c217821bf407ba1c35b8d0c5a7093&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/common/test_time_shifts.py
**Line:** 561:561
**Comment:**
*Custom Rule: Add an explicit return type annotation to this new test
function to comply with the rule requiring type hints.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42054&comment_hash=136cf16c29633e68b28b142e547795b7936848505c6a40cd3d5a0d89ef33c68b&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42054&comment_hash=136cf16c29633e68b28b142e547795b7936848505c6a40cd3d5a0d89ef33c68b&reaction=dislike'>๐</a>
--
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]
