sadpandajoe commented on code in PR #42054:
URL: https://github.com/apache/superset/pull/42054#discussion_r3593070973
##########
tests/unit_tests/utils/date_parser_tests.py:
##########
@@ -561,6 +564,70 @@ 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)
Review Comment:
Not changed: `dttm` is initialized with a `datetime(...)` literal, so an
annotation adds no information; the file's existing tests (e.g.
`test_get_past_or_future`) keep such locals unannotated.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
superset/utils/date_parser.py:
##########
@@ -95,9 +105,12 @@ def normalize_time_delta(human_readable: str) -> dict[str,
int]:
if not matched:
raise TimeDeltaAmbiguousError(human_readable)
- key = matched[2] + "s"
+ key = matched[2].lower() + "s"
Review Comment:
Not changed: `key` / `value` are locals whose types are exact from the
right-hand expressions (`str.lower() + "s"`, `int(...)`); mypy infers them
precisely, and the repo's type-hint rule targets signatures and module-level
names, not literal-initialized locals.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
tests/unit_tests/utils/date_parser_tests.py:
##########
@@ -561,6 +564,70 @@ 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)
Review Comment:
Not changed: same as above — a `datetime(...)`-literal local needs no
annotation, matching the file's existing style.
_🤖 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]
