SEPURI-SAI-KRISHNA commented on code in PR #45260:
URL: https://github.com/apache/superset/pull/45260#discussion_r4278475335
##########
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:
Taken, that reads better. Dropped the last sentence so the comment only
describes what the code does:
```python
# 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.
```
Also merged master in, which should clear the `check-openapi-spec-drift`
red. You were right that it was unrelated: that run is from before #45262
landed, and the check is green on master now.
--
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]
