EnxDev commented on code in PR #45260:
URL: https://github.com/apache/superset/pull/45260#discussion_r4276222747
##########
superset/utils/date_parser.py:
##########
@@ -905,15 +905,34 @@ def __init__(self, tokens: ParseResults) -> None:
def eval(self) -> datetime:
holiday = self.value[0].eval()
- dttm, country = [None, None]
- if len(self.value) >= 2:
- dttm = self.value[1].eval()
- if len(self.value) == 3:
- country = self.value[2]
+ dttm: datetime | None = None
+ country: str | None = None
+ # Both arguments after the holiday name are optional, so the second one
+ # is the datetime or the country depending on which were supplied. What
+ # tells them apart is the kind of argument: a datetime is always a
+ # function call (`datetime(...)`, `dateadd(...)`, ...) and a country is
+ # always quoted text. Reading the second argument as the datetime
+ # whatever it held left `holiday("Christmas Day", "UK")` taking `.year`
+ # off a string.
Review Comment:
Nit, take it or leave it. The last sentence describes the old bug rather
than the code, so it reads oddly once this is merged; the PR description
already covers it.
```suggestion
# always quoted text.
```
--
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]
