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

Incorrect offset for Berlin #89

Closed
abinoda opened this Issue Jun 7, 2018 · 5 comments

Comments

Projects
None yet
2 participants
@abinoda

abinoda commented Jun 7, 2018

Berlin's offset is currently +02:00 which will change to +01:00 Oct 28. However, the current offset returned is +01:00.

irb(main):015:0> tz = TZInfo::Timezone.get 'Europe/Berlin'
=> #<TZInfo::DataTimezone: Europe/Berlin>
irb(main):016:0> tz.current_period.utc_offset
=> 3600

I am using v1.2018.5 of tzinfo-data which appears to correctly define Berlin. See line 127 of https://github.com/tzinfo/tzinfo-data/blob/master/lib/tzinfo/data/definitions/Europe/Berlin.rb where it changes from offset+2 to offset+1 in October.

Also see rails/rails#33080

@abinoda

This comment has been minimized.

@abinoda abinoda closed this Jun 7, 2018

@abinoda abinoda reopened this Jun 7, 2018

@abinoda

This comment has been minimized.

abinoda commented Jun 7, 2018

Actually, I am still a bit confused. The comments in the code seem to imply that if I am using the tzinfo-data gem, utc_offset SHOULD take into account dst?

    # Note that zoneinfo files only include the value of {utc_total_offset} and
    # a DST flag. When using {DataSources::ZoneinfoDataSource}, the {utc_offset}
    # will be derived from changes to the UTC total offset and the DST flag. As
    # a consequence, {utc_total_offset} will always be correct, but {utc_offset}
    # may be inaccurate.
    #
    # If you require {utc_offset} to be accurate, install the tzinfo-data gem
    # and set {DataSources::RubyDataSource} as the {DataSource}.
@philr

This comment has been minimized.

Member

philr commented Jun 7, 2018

utc_offset is the base offset of the time zone. It does not take DST into account (regardless of whether tzinfo-data is being used) and is reasonably constant. For the offset including DST, you need the utc_total_offset attribute.

The reason for the comment you quoted is that, when using zoneinfo files as the data source, the value of utc_offset has to be derived. For a given time, zoneinfo files only indicate the utc_total_offset value and whether DST is being observed. TZInfo derives utc_offset by analysing transitions between periods of time where different utc_total_offset and/or different DST rules apply. This is correct for most time zones, most of the time, but could be inaccurate in certain situations.

The tzinfo-data gem includes the value of utc_offset. Nothing therefore needs to be derived when using the tzinfo-data gem as a data source.

@philr

This comment has been minimized.

Member

philr commented Jun 7, 2018

Duplicate of #55.

@abinoda

This comment has been minimized.

abinoda commented Jun 7, 2018

@philr Thank you for the in-depth response.

@abinoda abinoda closed this Jun 7, 2018

philr added a commit that referenced this issue Jun 7, 2018

Rename #utc_offset and #utc_total_offset.
The #utc_offset and #utc_total_offset attributes are often confused
(#55, #89). Rename as #base_utc_offset and #current_utc_offset
respectively.

Retain the old names as aliases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment