codeant-ai-for-open-source[bot] commented on code in PR #41489:
URL: https://github.com/apache/superset/pull/41489#discussion_r3488536237
##########
tests/unit_tests/models/helpers_test.py:
##########
@@ -2849,6 +2859,108 @@ def test_normalize_df_normalizes_legacy_time_column()
-> None:
assert result[DTTM_ALIAS][0].strftime("%Y-%m-%d") == "2020-01-01"
+def test_normalize_df_applies_offset_to_all_temporal_columns() -> None:
+ """Regression test for issue #23167: the dataset HOURS OFFSET must be
applied
+ to every temporal column a query returns, not only the selected time
column.
+ Two native-datetime temporal columns (neither declaring a
+ ``python_date_format``) must both be shifted by the dataset offset."""
+ import pandas as pd
+
+ from superset.models.helpers import ExploreMixin
+
+ created = MagicMock(
+ column_name="created",
+ is_dttm=True,
+ python_date_format=None,
+ datetime_format=None,
+ )
+ expired = MagicMock(
+ column_name="expired",
+ is_dttm=True,
+ python_date_format=None,
+ datetime_format=None,
+ )
+ columns = {"created": created, "expired": expired}
+
+ datasource = MagicMock()
+ datasource.offset = 4
+ datasource.enforce_numerical_metrics = False
+ datasource.columns = list(columns.values())
+ datasource.get_column = lambda name: columns.get(name)
+ for method in (
+ "_python_date_format",
+ "_collect_dttm_labels",
+ "_offset_only_dttm_cols",
+ "normalize_df",
+ ):
+ setattr(datasource, method, getattr(ExploreMixin,
method).__get__(datasource))
+
+ query_object = MagicMock()
+ query_object.columns = ["created", "expired"]
+ query_object.granularity = None
+ query_object.time_shift = None
+
+ df = pd.DataFrame(
+ {
+ "created": pd.to_datetime(["2020-01-01 00:00:00", "2020-01-02
00:00:00"]),
+ "expired": pd.to_datetime(["2020-06-01 12:00:00", "2020-06-02
12:00:00"]),
+ }
+ )
+
+ result = datasource.normalize_df(df, query_object)
+
+ assert (
+ result["created"].tolist()
+ == pd.to_datetime(["2020-01-01 04:00:00", "2020-01-02
04:00:00"]).tolist()
+ )
+ assert (
+ result["expired"].tolist()
+ == pd.to_datetime(["2020-06-01 16:00:00", "2020-06-02
16:00:00"]).tolist()
+ )
+
+
+def test_normalize_df_offset_skips_unconfigured_integer_temporal_columns() ->
None:
+ """The offset extension for native-datetime temporal columns must not
touch a
+ temporal column whose values arrive as plain integers with no declared
+ format: such a column cannot be safely interpreted as a datetime, so it is
+ left untouched rather than reinterpreted as nanoseconds (see issue
#23167)."""
+ import pandas as pd
+ from pandas.api.types import is_datetime64_any_dtype
+
+ from superset.models.helpers import ExploreMixin
+
+ int_col = MagicMock(
+ column_name="ts",
+ is_dttm=True,
+ python_date_format=None,
+ datetime_format=None,
+ )
+ datasource = MagicMock()
+ datasource.offset = 4
+ datasource.enforce_numerical_metrics = False
+ datasource.columns = [int_col]
+ datasource.get_column = lambda name: {"ts": int_col}.get(name)
Review Comment:
**Suggestion:** Use an explicitly typed local function instead of this
lambda so the input argument and optional return type are annotated.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The assigned lambda omits both the input annotation and the optional return
type annotation. Since this modified code can be annotated, it violates the
stated Python type-hint rule.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8086a409a2d74094a3f106c55cc9bba7&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=8086a409a2d74094a3f106c55cc9bba7&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/models/helpers_test.py
**Line:** 2942:2942
**Comment:**
*Custom Rule: Use an explicitly typed local function instead of this
lambda so the input argument and optional return type are annotated.
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%2F41489&comment_hash=cb2b28160019a4aa057804d6fb19261084e88a3b1765405396caedbbe6c15362&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41489&comment_hash=cb2b28160019a4aa057804d6fb19261084e88a3b1765405396caedbbe6c15362&reaction=dislike'>👎</a>
##########
tests/unit_tests/models/helpers_test.py:
##########
@@ -2849,6 +2859,108 @@ def test_normalize_df_normalizes_legacy_time_column()
-> None:
assert result[DTTM_ALIAS][0].strftime("%Y-%m-%d") == "2020-01-01"
+def test_normalize_df_applies_offset_to_all_temporal_columns() -> None:
+ """Regression test for issue #23167: the dataset HOURS OFFSET must be
applied
+ to every temporal column a query returns, not only the selected time
column.
+ Two native-datetime temporal columns (neither declaring a
+ ``python_date_format``) must both be shifted by the dataset offset."""
+ import pandas as pd
+
+ from superset.models.helpers import ExploreMixin
+
+ created = MagicMock(
+ column_name="created",
+ is_dttm=True,
+ python_date_format=None,
+ datetime_format=None,
+ )
+ expired = MagicMock(
+ column_name="expired",
+ is_dttm=True,
+ python_date_format=None,
+ datetime_format=None,
+ )
+ columns = {"created": created, "expired": expired}
+
+ datasource = MagicMock()
+ datasource.offset = 4
+ datasource.enforce_numerical_metrics = False
+ datasource.columns = list(columns.values())
+ datasource.get_column = lambda name: columns.get(name)
Review Comment:
**Suggestion:** Replace this untyped lambda with a small local function that
includes explicit parameter and return type annotations, then assign that
function to `datasource.get_column`. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The lambda assigned to `datasource.get_column` has no parameter or return
type annotations. This is new Python code in the modified hunk and fits the
rule requiring type hints for annotatable functions/callables.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=be34d701be0b4618b6636c80f75499ac&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=be34d701be0b4618b6636c80f75499ac&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/models/helpers_test.py
**Line:** 2889:2889
**Comment:**
*Custom Rule: Replace this untyped lambda with a small local function
that includes explicit parameter and return type annotations, then assign that
function to `datasource.get_column`.
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%2F41489&comment_hash=aeb80d168b4d5d0eb651d9117ac4aa654bb3ab1b05ee2d9ad411a6d3ef0c98db&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41489&comment_hash=aeb80d168b4d5d0eb651d9117ac4aa654bb3ab1b05ee2d9ad411a6d3ef0c98db&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]
