Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Timezone - Error in Offset #19

Open
codeyash opened this issue Oct 28, 2023 · 11 comments
Open

Timezone - Error in Offset #19

codeyash opened this issue Oct 28, 2023 · 11 comments

Comments

@codeyash
Copy link

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
Copy link
Owner

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
Copy link
Author

codeyash commented Oct 28, 2023

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
Copy link
Owner

Yuri6037 commented Oct 28, 2023

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
Copy link
Author

codeyash commented Oct 29, 2023

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
Copy link
Owner

Yuri6037 commented Oct 29, 2023

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
Copy link
Author

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
Copy link
Owner

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.

@Yuri6037
Copy link
Owner

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
Copy link
Author

Sure!

@codeyash
Copy link
Author

codeyash commented Oct 29, 2023

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
Copy link
Owner

Yuri6037 commented Oct 29, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants