codeant-ai-for-open-source[bot] commented on code in PR #42054:
URL: https://github.com/apache/superset/pull/42054#discussion_r3598262049
##########
tests/unit_tests/utils/date_parser_tests.py:
##########
@@ -561,6 +565,109 @@ def test_get_past_or_future() -> None:
assert get_past_or_future("3 month", dttm) == datetime(2020, 5, 29)
+def test_get_past_or_future_quarters() -> None:
+ # parsedatetime has no notion of quarters (it would leave the source time
+ # unchanged); quarter phrases are rewritten to months before parsing
+ dttm = datetime(2024, 5, 15, 10, 30, 45)
+ assert get_past_or_future("1 quarter ago", dttm) == datetime(
+ 2024, 2, 15, 10, 30, 45
+ )
+ assert get_past_or_future("2 quarters ago", dttm) == datetime(
+ 2023, 11, 15, 10, 30, 45
+ )
+ assert get_past_or_future("1 quarter later", dttm) == datetime(
+ 2024, 8, 15, 10, 30, 45
+ )
+ assert get_past_or_future("1 QUARTER ago", dttm) == datetime(
+ 2024, 2, 15, 10, 30, 45
+ )
+ # absurdly long digit runs are not rewritten (bounded to keep matching
+ # linear and the month count small); they parse like any other
+ # unintelligible phrase, i.e. the source time comes back unchanged
+ assert get_past_or_future("0" * 100_000 + " quarters ago", dttm) == dttm
+
+
+def test_parse_human_timedelta_unparseable_is_zero() -> None:
+ # the parser leaves unparseable phrases as the source time, so their
+ # delta is zero; a zero delta alone cannot distinguish them from
+ # legitimate zero-shift phrases (see is_parseable_human_timedelta)
+ dttm = datetime(2024, 5, 15)
+ assert parse_human_timedelta("not a real offset", dttm) == timedelta(0)
+
+
+def test_is_parseable_human_timedelta() -> None:
+ assert is_parseable_human_timedelta("1 week ago")
+ assert is_parseable_human_timedelta("one year ago")
+ assert is_parseable_human_timedelta("1 quarter ago")
+ # zero-shift phrases parse successfully even though their delta is zero
+ assert is_parseable_human_timedelta("0 days ago")
+ assert not is_parseable_human_timedelta("not a real offset")
+ assert not is_parseable_human_timedelta("invalid-date-range")
+ assert not is_parseable_human_timedelta("")
+ assert not is_parseable_human_timedelta(None)
+ # absurdly long digit runs are not rewritten to months; they are
+ # unparseable like any other unintelligible phrase
+ assert not is_parseable_human_timedelta("9" * 100_000 + " quarters ago")
+
+
+def test_is_constant_human_timedelta() -> None:
+ # phrases that shift every source time by the same amount
+ assert is_constant_human_timedelta("1 week ago")
+ assert is_constant_human_timedelta("one year ago")
+ assert is_constant_human_timedelta("1 quarter ago")
+ assert is_constant_human_timedelta("2 days later")
+ # a zero shift is still a constant one
+ assert is_constant_human_timedelta("0 days ago")
+ # anchors resolve to a fixed point instead: how far they move a source
+ # time depends on where in the day that source time falls
+ assert not is_constant_human_timedelta("yesterday")
+ assert not is_constant_human_timedelta("last month")
+ assert not is_constant_human_timedelta("noon")
+ # a phrase nothing can parse is not a delta either
+ assert not is_constant_human_timedelta("not a real offset")
+ assert not is_constant_human_timedelta("")
+ assert not is_constant_human_timedelta(None)
+
+
+def test_is_constant_human_timedelta_matches_applied_shift() -> None:
+ """
+ The probes stand in for real source timestamps, so the verdict has to hold
+ for source times they never looked at: an accepted phrase shifts arbitrary
+ timestamps equally, a rejected one does not.
+ """
+ sources = [
+ datetime(2021, 3, 14, 2, 30),
+ datetime(2021, 11, 7, 1, 30),
+ datetime(2024, 2, 29, 23, 59, 59),
+ datetime(2020, 1, 1, 0, 0),
+ ]
+
+ deltas = {get_past_or_future("1 week ago", src) - src for src in sources}
+ assert deltas == {timedelta(days=-7)}
Review Comment:
**Suggestion:** Add an explicit type annotation to this newly added computed
variable so its set element type is clear to static analyzers. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
The new set-valued variable is inferred by Python but can be explicitly
annotated. Since the rule requires type hints on relevant new variables that
can be annotated, this omission is a real violation.
</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=ac8927a4a19d408bb627dc5dd26bac1a&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=ac8927a4a19d408bb627dc5dd26bac1a&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/utils/date_parser_tests.py
**Line:** 645:646
**Comment:**
*Custom Rule: Add an explicit type annotation to this newly added
computed variable so its set element type is clear to static analyzers.
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=6bc3c9fc63b2038fb327788ccaed79e63e57aab99661b19b419b78c8318e58fa&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42054&comment_hash=6bc3c9fc63b2038fb327788ccaed79e63e57aab99661b19b419b78c8318e58fa&reaction=dislike'>๐</a>
##########
tests/unit_tests/utils/date_parser_tests.py:
##########
@@ -561,6 +565,109 @@ def test_get_past_or_future() -> None:
assert get_past_or_future("3 month", dttm) == datetime(2020, 5, 29)
+def test_get_past_or_future_quarters() -> None:
+ # parsedatetime has no notion of quarters (it would leave the source time
+ # unchanged); quarter phrases are rewritten to months before parsing
+ dttm = datetime(2024, 5, 15, 10, 30, 45)
+ assert get_past_or_future("1 quarter ago", dttm) == datetime(
+ 2024, 2, 15, 10, 30, 45
+ )
+ assert get_past_or_future("2 quarters ago", dttm) == datetime(
+ 2023, 11, 15, 10, 30, 45
+ )
+ assert get_past_or_future("1 quarter later", dttm) == datetime(
+ 2024, 8, 15, 10, 30, 45
+ )
+ assert get_past_or_future("1 QUARTER ago", dttm) == datetime(
+ 2024, 2, 15, 10, 30, 45
+ )
+ # absurdly long digit runs are not rewritten (bounded to keep matching
+ # linear and the month count small); they parse like any other
+ # unintelligible phrase, i.e. the source time comes back unchanged
+ assert get_past_or_future("0" * 100_000 + " quarters ago", dttm) == dttm
+
+
+def test_parse_human_timedelta_unparseable_is_zero() -> None:
+ # the parser leaves unparseable phrases as the source time, so their
+ # delta is zero; a zero delta alone cannot distinguish them from
+ # legitimate zero-shift phrases (see is_parseable_human_timedelta)
+ dttm = datetime(2024, 5, 15)
+ assert parse_human_timedelta("not a real offset", dttm) == timedelta(0)
+
+
+def test_is_parseable_human_timedelta() -> None:
+ assert is_parseable_human_timedelta("1 week ago")
+ assert is_parseable_human_timedelta("one year ago")
+ assert is_parseable_human_timedelta("1 quarter ago")
+ # zero-shift phrases parse successfully even though their delta is zero
+ assert is_parseable_human_timedelta("0 days ago")
+ assert not is_parseable_human_timedelta("not a real offset")
+ assert not is_parseable_human_timedelta("invalid-date-range")
+ assert not is_parseable_human_timedelta("")
+ assert not is_parseable_human_timedelta(None)
+ # absurdly long digit runs are not rewritten to months; they are
+ # unparseable like any other unintelligible phrase
+ assert not is_parseable_human_timedelta("9" * 100_000 + " quarters ago")
+
+
+def test_is_constant_human_timedelta() -> None:
+ # phrases that shift every source time by the same amount
+ assert is_constant_human_timedelta("1 week ago")
+ assert is_constant_human_timedelta("one year ago")
+ assert is_constant_human_timedelta("1 quarter ago")
+ assert is_constant_human_timedelta("2 days later")
+ # a zero shift is still a constant one
+ assert is_constant_human_timedelta("0 days ago")
+ # anchors resolve to a fixed point instead: how far they move a source
+ # time depends on where in the day that source time falls
+ assert not is_constant_human_timedelta("yesterday")
+ assert not is_constant_human_timedelta("last month")
+ assert not is_constant_human_timedelta("noon")
+ # a phrase nothing can parse is not a delta either
+ assert not is_constant_human_timedelta("not a real offset")
+ assert not is_constant_human_timedelta("")
+ assert not is_constant_human_timedelta(None)
+
+
+def test_is_constant_human_timedelta_matches_applied_shift() -> None:
+ """
+ The probes stand in for real source timestamps, so the verdict has to hold
+ for source times they never looked at: an accepted phrase shifts arbitrary
+ timestamps equally, a rejected one does not.
+ """
+ sources = [
+ datetime(2021, 3, 14, 2, 30),
+ datetime(2021, 11, 7, 1, 30),
+ datetime(2024, 2, 29, 23, 59, 59),
+ datetime(2020, 1, 1, 0, 0),
+ ]
+
+ deltas = {get_past_or_future("1 week ago", src) - src for src in sources}
+ assert deltas == {timedelta(days=-7)}
+
+ anchored = {get_past_or_future("yesterday", src) - src for src in sources}
+ assert len(anchored) > 1
Review Comment:
**Suggestion:** Add an explicit type annotation to this newly introduced
variable to make the inferred set type explicit and rule-compliant.
[custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This is another newly introduced local variable with an inferable set type
that could be annotated explicitly. The absence of a type hint violates the
Python type-hint rule.
</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=5a35b9783bcb458c95a1e84f0fb369a9&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=5a35b9783bcb458c95a1e84f0fb369a9&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/utils/date_parser_tests.py
**Line:** 648:649
**Comment:**
*Custom Rule: Add an explicit type annotation to this newly introduced
variable to make the inferred set type explicit and rule-compliant.
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=ba5a9bc3d499cdb4e8cc249be81c40d1ad531f6f793cb79ca55300964340f288&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42054&comment_hash=ba5a9bc3d499cdb4e8cc249be81c40d1ad531f6f793cb79ca55300964340f288&reaction=dislike'>๐</a>
##########
tests/unit_tests/utils/date_parser_tests.py:
##########
@@ -561,6 +565,109 @@ def test_get_past_or_future() -> None:
assert get_past_or_future("3 month", dttm) == datetime(2020, 5, 29)
+def test_get_past_or_future_quarters() -> None:
+ # parsedatetime has no notion of quarters (it would leave the source time
+ # unchanged); quarter phrases are rewritten to months before parsing
+ dttm = datetime(2024, 5, 15, 10, 30, 45)
+ assert get_past_or_future("1 quarter ago", dttm) == datetime(
+ 2024, 2, 15, 10, 30, 45
+ )
+ assert get_past_or_future("2 quarters ago", dttm) == datetime(
+ 2023, 11, 15, 10, 30, 45
+ )
+ assert get_past_or_future("1 quarter later", dttm) == datetime(
+ 2024, 8, 15, 10, 30, 45
+ )
+ assert get_past_or_future("1 QUARTER ago", dttm) == datetime(
+ 2024, 2, 15, 10, 30, 45
+ )
+ # absurdly long digit runs are not rewritten (bounded to keep matching
+ # linear and the month count small); they parse like any other
+ # unintelligible phrase, i.e. the source time comes back unchanged
+ assert get_past_or_future("0" * 100_000 + " quarters ago", dttm) == dttm
+
+
+def test_parse_human_timedelta_unparseable_is_zero() -> None:
+ # the parser leaves unparseable phrases as the source time, so their
+ # delta is zero; a zero delta alone cannot distinguish them from
+ # legitimate zero-shift phrases (see is_parseable_human_timedelta)
+ dttm = datetime(2024, 5, 15)
+ assert parse_human_timedelta("not a real offset", dttm) == timedelta(0)
+
+
+def test_is_parseable_human_timedelta() -> None:
+ assert is_parseable_human_timedelta("1 week ago")
+ assert is_parseable_human_timedelta("one year ago")
+ assert is_parseable_human_timedelta("1 quarter ago")
+ # zero-shift phrases parse successfully even though their delta is zero
+ assert is_parseable_human_timedelta("0 days ago")
+ assert not is_parseable_human_timedelta("not a real offset")
+ assert not is_parseable_human_timedelta("invalid-date-range")
+ assert not is_parseable_human_timedelta("")
+ assert not is_parseable_human_timedelta(None)
+ # absurdly long digit runs are not rewritten to months; they are
+ # unparseable like any other unintelligible phrase
+ assert not is_parseable_human_timedelta("9" * 100_000 + " quarters ago")
+
+
+def test_is_constant_human_timedelta() -> None:
+ # phrases that shift every source time by the same amount
+ assert is_constant_human_timedelta("1 week ago")
+ assert is_constant_human_timedelta("one year ago")
+ assert is_constant_human_timedelta("1 quarter ago")
+ assert is_constant_human_timedelta("2 days later")
+ # a zero shift is still a constant one
+ assert is_constant_human_timedelta("0 days ago")
+ # anchors resolve to a fixed point instead: how far they move a source
+ # time depends on where in the day that source time falls
+ assert not is_constant_human_timedelta("yesterday")
+ assert not is_constant_human_timedelta("last month")
+ assert not is_constant_human_timedelta("noon")
+ # a phrase nothing can parse is not a delta either
+ assert not is_constant_human_timedelta("not a real offset")
+ assert not is_constant_human_timedelta("")
+ assert not is_constant_human_timedelta(None)
+
+
+def test_is_constant_human_timedelta_matches_applied_shift() -> None:
+ """
+ The probes stand in for real source timestamps, so the verdict has to hold
+ for source times they never looked at: an accepted phrase shifts arbitrary
+ timestamps equally, a rejected one does not.
+ """
+ sources = [
+ datetime(2021, 3, 14, 2, 30),
+ datetime(2021, 11, 7, 1, 30),
+ datetime(2024, 2, 29, 23, 59, 59),
+ datetime(2020, 1, 1, 0, 0),
+ ]
Review Comment:
**Suggestion:** Add an explicit type annotation to this collection variable
to satisfy the type-hint requirement for relevant new variables. [custom_rule]
**Severity Level:** Minor ๐งน
<details>
<summary><b>Why it matters? โญ </b></summary>
This is a newly added local variable in Python test code, and it lacks an
explicit type hint even though its list element type is clearly annotatable.
This matches the type-hint rule for relevant variables.
</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=ec6091267d454c0d8e66d983851461a0&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=ec6091267d454c0d8e66d983851461a0&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/utils/date_parser_tests.py
**Line:** 638:643
**Comment:**
*Custom Rule: Add an explicit type annotation to this collection
variable to satisfy the type-hint requirement for relevant new variables.
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=63fd2d51318ff704568b1c95bdba0fdad4f47674d2dd0edc7ad882ca09999b74&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42054&comment_hash=63fd2d51318ff704568b1c95bdba0fdad4f47674d2dd0edc7ad882ca09999b74&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]
