search

sopel-irc / sopel-weather

A working re-implementation of the weather plugin for Sopel
MIT License
6 stars 7 forks source link

Sunrise and sunset are displayed in timestamp format #30

Closed sukiletxe closed 3 years ago

sukiletxe commented 3 years ago

I'm using openweathermap as my weather provider, and the sunrise and sunset information are displayed in timestamp format (not sure if seconds or milliseconds). I don't know if the proper way to fix this would be to do a strptime / strftime round-tripor if there's a better way.

RustyBower commented 3 years ago

Can you copy paste the command and output you're getting? Also what your weather section looks like in settings (please comment out your API key)

On Mon, Jun 14, 2021 at 8:40 AM Sukil Etxenike @.***> wrote:

I'm using openweathermap as my weather provider, and the sunrise and sunset information are displayed in timestamp format (not sure if seconds or milliseconds). I don't know if the proper way to fix this would be to do a strptime / strftime round-tripor if there's a better way.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/sopel-irc/sopel-weather/issues/30, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA6JAHNZHZOFN7LEN5ZGPSDTSYBDJANCNFSM46VIH6EQ .

sukiletxe commented 3 years ago 
.w howston
Sarnia, Ontario, CA: 19°C (67°F), Clouds, Humidity: 74%, Sunrise: 1623664189 Sunset: 1623719401, Gentle breeze 4.9m/s (↙)

geocoords_provider = locationiq
weather_provider = openweathermap
sunrise_sunset = true
nick_lookup = true
dgw commented 3 years ago

OWM docs for the "onecall" API this plugin uses say that sunrise/sunset are Unix timestamps in UTC. I can't access the Dark Sky API documentation for comparison, as I don't have an account.

Regardless of Dark Sky, the obvious solution for OWM is converting the value to a datetime using datetime.fromtimestamp() and formatting it with sopel.tools.time.format_time().

If it's supposed to be the weather provider's job to format the value into a human-friendly string, then the place to do that is here:

https://github.com/sopel-irc/sopel-weather/blob/3ae16772e712c48c6b6db4b936359fcb93dea888/sopel_modules/weather/providers/weather/openweathermap.py#L64-L66

sukiletxe commented 3 years ago

Seems both use timestamps in seconds. Should I create only two functions in weather.py (just like get_temp, for example, one for sunrise and another one for sunset), or should I split the logic just in case incompatible providers are added?

RustyBower commented 3 years ago

If you wanna put in a PR to fix this, I would have a convert_timestamp (or something) function, and just call those on sunrise/sunset fields. If there is a provider that doesn't respond with UNIX timestamps, we can cross that bridge, but afaik openweathermap tries to emulate darksky as much as possible

sukiletxe commented 3 years ago

Woops, didn't think of that ¬¬ . OK, will do.