add timezone to ee.Date conversion to datetime #423
Conversation
|
I noticed something similar while I was using it, but my solution was to hardcode the problem 🤣 |
| @@ -88,9 +89,15 @@ def now(cls) -> ee.Date: | |||
| """ | |||
| return ee.Date(datetime.now().isoformat()) | |||
|
|
|||
| def to_datetime(self) -> datetime: | |||
| def to_datetime(self, tz=zoneinfo.ZoneInfo("UTC")) -> datetime: | |||
There was a problem hiding this comment.
I think that it would be a good idea left the tz variable as a string, following the same convention of the ee.Date constructor. And maybe, put a link to this page in the documentation: https://www.joda.org/joda-time/timezones.html to know the possible accepted strings.
Could you add a typing for this variable? If you decide to leave it as a ZoneInfo, type it with that class. Or in the second option, simply leave it as an str.
| @@ -106,7 +113,8 @@ def to_datetime(self) -> datetime: | |||
| d.strftime('%Y-%m-%d') | |||
|
|
|||
| """ | |||
| return datetime.fromtimestamp(self._obj.millis().getInfo() / 1000.0) | |||
| datestr = self._obj.format(None, str(tz)).getInfo() | |||
| return datetime.fromisoformat(datestr) | |||
There was a problem hiding this comment.
I think the formatting of an ee.Date and the setting of a datetime from a string are both very slow. Maybe you could do some performance testing with a code like this:
date_time = datetime.fromtimestamp(self._obj.millis().getInfo() / 1000.0)
date_time = date_time.replace(tzinfo=ZoneInfo("UTC"))
date_time = date_time.astimezone(tz)(I got the code from this answer on stackoverflow, it may be wrong).
| the timezone from it, thus it must be passed as argument. | ||
|
|
||
| Args: | ||
| tz: time zone. Defaults to UTC. |
There was a problem hiding this comment.
This is a more subjective opinion, but could you make the reference to ee.Date by replacing it with the following code?
py:class:`ee.Date`This is in order to make it clickeable to the original documentation.
And, could you leave the documentation for the tz variable starting with a capital letter? That is, instead of "time zone" write "Time zone".
Luckily I live in a different time zone than UTC and tests for this method was failing due to it.
I follow the GEE pattern as
ee.Date.formatmethod, to which user can pass a time zone and get the formatted representation of that date in the specified time zone (https://code.earthengine.google.com/91d328471a218815afda4309021dc68a)