sadpandajoe commented on code in PR #42054:
URL: https://github.com/apache/superset/pull/42054#discussion_r3598284673
##########
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:
Not changed: `sources`, `deltas`, and `anchored` are locals whose types are
fully inferred from their right-hand sides (`list[datetime]`,
`set[timedelta]`), so mypy already checks them and an inline annotation would
only restate the literal. The file follows this convention — 206 unannotated
locals vs 5 annotated, and those 5 are bare declarations before conditional
assignment, where inference genuinely is not possible. Happy to annotate if you
consider these relevant new variables under the rule.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
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:
Not changed: `sources`, `deltas`, and `anchored` are locals whose types are
fully inferred from their right-hand sides (`list[datetime]`,
`set[timedelta]`), so mypy already checks them and an inline annotation would
only restate the literal. The file follows this convention — 206 unannotated
locals vs 5 annotated, and those 5 are bare declarations before conditional
assignment, where inference genuinely is not possible. Happy to annotate if you
consider these relevant new variables under the rule.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
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:
Not changed: `sources`, `deltas`, and `anchored` are locals whose types are
fully inferred from their right-hand sides (`list[datetime]`,
`set[timedelta]`), so mypy already checks them and an inline annotation would
only restate the literal. The file follows this convention — 206 unannotated
locals vs 5 annotated, and those 5 are bare declarations before conditional
assignment, where inference genuinely is not possible. Happy to annotate if you
consider these relevant new variables under the rule.
_🤖 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]
