aminghadersohi commented on code in PR #44620:
URL: https://github.com/apache/superset/pull/44620#discussion_r4107259247
##########
superset/mcp_service/chart/chart_helpers.py:
##########
@@ -151,7 +151,7 @@ def find_chart_by_identifier(
{"query_options": query_options} if query_options is not None else {}
)
if isinstance(identifier, int) or (
- isinstance(identifier, str) and identifier.isdigit()
+ isinstance(identifier, str) and identifier.isdecimal()
Review Comment:
Reverting all 13 `mcp_service` sites to `isdigit()` leaves
`tests/unit_tests/mcp_service/` at 2198 passed / 3 skipped — identical to
baseline, so none is pinned. The three DAO/Slice tests do kill their mutants;
one parametrized `"²"` case over these resolvers would close the rest.
##########
superset/utils/date_parser.py:
##########
@@ -349,7 +349,7 @@ def handle_nth_of(
"DATETRUNC(..., year) : DATEADD(DATETRUNC(..., year), 1, week)"
"""
# Convert ordinal to number
- n = ORDINAL_MAP.get(ordinal.lower(), int(ordinal) if ordinal.isdigit()
else 1)
+ n = ORDINAL_MAP.get(ordinal.lower(), int(ordinal) if ordinal.isdecimal()
else 1)
Review Comment:
codeant is wrong here: the `dict.get` default is eager, but the ternary
guards `int()` — `"first".isdecimal()` is False so it yields `1`, measured
identical under both predicates. All three callers capture `(first|1st)`, both
`ORDINAL_MAP` keys, so `int(ordinal)` is unreachable.
--
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]
