Yuri6037/time-tz

Timezone - Error in Offset

Opened this issue · 11 comments

codeyash commented 
let timezone_ = timezones::get_by_name("America/New_York").unwrap();

let offset1 = timezone_.get_offset_primary().to_utc(); // -4.56 WRONG
        
let d = OffsetDateTime::now_utc();
let dt4= PrimitiveDateTime::new(d.date(), d.time()).assume_timezone_utc(timezone_);
let offset2 = dt4.offset(); // -4.00 CORRECT

assert(offset1, offset2)

Both offsets are different. Am I doing wrong or is this a bug?

Yuri6037 commented

get_offset_primary does not return the current offset from UTC instead it returns a default offset (check the doc for the function). Using assume_timezone_utc returns the current offset from UTC matching the given timestamp (in this case the timestamp is obtained from now_utc, which is used correctly for this function).

Note that if you used a non-UTC primitive date time with assume_timezone_utc you could get a broken date time, typically one which does not care of DST.

codeyash commented

Thanks for the reply.

let timezone = timezones::get_by_name("America/New_York").unwrap();
let d = OffsetDateTime::now_utc();
let dt= PrimitiveDateTime::new(d.date(), d.time()).assume_timezone_utc(timezone);
let offset = dt.offset(); // -4.00 CORRECT

What is the cleanest way of achieving this?

My case is super simple: Get a current OffSetDateTime for a provided timezone.

// Similar to 
OffsetDatetime::now_utc()

// May be like 
OffsetDatetimeExt::now(timezone)
Yuri6037 commented

Thanks for the reply.

let timezone = timezones::get_by_name("America/New_York").unwrap();
let d = OffsetDateTime::now_utc();
let dt= PrimitiveDateTime::new(d.date(), d.time()).assume_timezone_utc(timezone);
let offset = dt.offset(); // -4.00 CORRECT

What is the cleanest way of achieving this?

My case is super simple: Get a current OffSetDateTime for a provided timezone.

// Similar to 
OffsetDatetime::now_utc()

// May be like 
OffsetDatetimeExt::now(timezone)

If you don't need more calculations with the date time once converted to the target timezone, you can do the following:

OffsetDateTime::now_utc().to_timezone(timezones::db::america::NEW_YORK)

If you need more calculations in the target timezone, you can use ZonedDateTime which is a new feature only available in the current master branch.

codeyash commented

Thanks for your help and your wonderful library :)

// Accepted for now
let timezone = timezones::get_by_name("America/New_York").unwrap();
let d = OffsetDateTime::now_utc().to_timezone(timezone);



// Expected may be like a feature
// "America/New_York" can be a variable.
let d = OffsetDateTime::now_utc().to_timezone("America/New_York");

// How to support multi type parameter for same fn
// https://blog.rust-lang.org/2015/05/11/traits.html

``
Yuri6037 commented

Thanks for your help and your wonderful library :)

// Accepted for now
let timezone = timezones::get_by_name("America/New_York").unwrap();
let d = OffsetDateTime::now_utc().to_timezone(timezone);



// Expected may be like a feature
// "America/New_York" can be a variable.
let d = OffsetDateTime::now_utc().to_timezone("America/New_York");

// How to support multi type parameter for same fn
// https://blog.rust-lang.org/2015/05/11/traits.html

``

I'm happy that you could find what you needed.

What you're suggesting about the API is not really possible as this would require support for overload on the type of argument, moreover you don't handle the case where get_by_name returns None. The function may return None if it cannot find the timezone you're requesting.

By the way, why don't you use the timezone types directly?

codeyash commented

By the way, why don't you use the timezone types directly? - Timezone is saved in DB. It's a string.

// I agree that None can be produced based on input
// How about this?
OffsetDateTime::now_utc().try_timezone("America/New_York"); -> Result<OffsetDateTime, err>
Yuri6037 commented

By the way, why don't you use the timezone types directly? - Timezone is saved in DB. It's a string.

// I agree that None can be produced based on input
// How about this?
OffsetDateTime::now_utc().try_timezone("America/New_York"); -> Result<OffsetDateTime, err>

I like the idea of a try_to_timezone. I was also thinking of something a bit more involved by creating a new ToTimezone<T> trait.

  • 👍1
Yuri6037 commented

Thanks for your help and your wonderful library :)

// Accepted for now
let timezone = timezones::get_by_name("America/New_York").unwrap();
let d = OffsetDateTime::now_utc().to_timezone(timezone);



// Expected may be like a feature
// "America/New_York" can be a variable.
let d = OffsetDateTime::now_utc().to_timezone("America/New_York");

// How to support multi type parameter for same fn
// https://blog.rust-lang.org/2015/05/11/traits.html

``

Can you try the new to_timezone from the master branch? I've done quite a bit of refactor with the to_timezone function.

codeyash commented

Sure!

codeyash commented 
time-tz= { git = "https://github.com/Yuri6037/time-tz", version = "3.0.0-rc.1.0.0", features = ["db_impl"] }

//had to include ToTimezone and db_impl is required for Tz
use time_tz::{ToTimezone, timezones, TimeZone, Offset, Tz};

Tried with this and code is working as expected. I see no changes except in use statement.

Yuri6037 commented 
time-tz= { git = "https://github.com/Yuri6037/time-tz", version = "3.0.0-rc.1.0.0", features = ["db_impl"] }

//had to include ToTimezone and db_impl is required for Tz
use time_tz::{ToTimezone, timezones, TimeZone, Offset, Tz};

Tried with this and code is working as expected. I see no changes except in use statement.

The new use statement is expected as now the function to_timezone has been extracted to its own trait.

By the way try removing the import for Tz as you shouldn't need it. It's supposed to be hidden implementation details.

  • 👍1