floraison / et-orbi

Time zones for fugit and rufus-scheduler. Urbi et Orbi.
MIT License
24 stars 11 forks source link

EtOrbi::EoTime#to_time ignores timezone, produces wrong values #2

Closed ramfjord closed 7 years ago

ramfjord commented 7 years ago

Should be pretty self explanatory - I believe I'm using the most recent version. Below is an illustrative snippet:

require 'et_orbi'
# => true
EtOrbi::VERSION
# => "1.0.5"
puts EtOrbi.platform_info
# (etz:nil,tnz:"UTC",tzid:nil,rv:"1.9.3",rp:"x86_64-linux",eov:"1.0.5",rorv:nil,astz:nil,debian:nil,centos:nil,osx:"UTC")
# => nil
my_time = EtOrbi.make_time(2017, 1, 31, 12, 'Europe/Moscow')
# => 2017-01-31 12:00:00 +0300
my_time.to_time
# => 2017-01-31 12:00:00 UTC
my_time.to_i
# => 1485853200
my_time.to_time.to_i
# => 1485864000

It might make sense to use the system timezone for #to_time, but I would have expected it to at act more like:

Time.at(my_time.to_f)

Which incidentally is what I'm currently using as a workaround. For a bit of background, I'm using this through the Rufus::Scheduler gem.

jmettraux commented 7 years ago

Hello,

thanks for engaging the conversation on this.

It behaves as expected: https://github.com/floraison/et-orbi/blob/faf733c18cc83b694c7309153a5925d0141bcce8/lib/et-orbi.rb#L296-L303

I will add a EtOrbi::EoTime#to_local_time. What do you think?

jmettraux commented 7 years ago

Closing, since EtOrbi::EoTime#to_time behaves as specified and produces the right values.

ramfjord commented 7 years ago

Fair enough - it does say it will be returned in UTC, shouldn't it return the correct time in UTC at least? E.G.

my_time = EtOrbi.make_time(2017, 1, 31, 12, 'Europe/Moscow')
# => 2017-01-31 12:00:00 +0300
my_time.to_time
# => 2017-01-31 09:00:00 UTC
jmettraux commented 7 years ago

It is the correct time:

ENV['TZ'] = 'Europe/Moscow' 

t = Time.new(2017, 1, 31, 12)                                                   

p t      # => 2017-01-31 12:00:00 +0300                                         
p t.utc  # => 2017-01-31 09:00:00 UTC                                           
jmettraux commented 7 years ago

OK,

do not use EtOrbi::EoTime#to_time, I will make it protected, I should not have left it public. I will provide a to_t that will return the result of Time.at(@seconds).

Thanks.

jmettraux commented 7 years ago

Please note that ::EtOrbi::EoTime responds to most of the methods that ::Time responds to.

Feel free to open an issue if a method is missing and I will add it or suggest a workaround.

ramfjord commented 7 years ago

Alright, thanks! Just wanted to inform you of an issue people might find. I'm happy using my workaround for now.