Add IANA/BCP47 time zone name mappings #3499

sffc · 2023-06-07T06:04:33Z

The data structure is based heavily on the property names.

sffc · 2023-06-07T06:29:08Z

provider/repodata/data/json/time_zone/iana_to_bcp47@1/und.json

@@ -0,0 +1,599 @@
+{
+  "map": {
+    "Africa/Abidjan": "ciabj",


Note: This should benefit a bunch from AsciiTrie.

Manishearth · 2023-06-07T07:23:31Z

components/timezone/src/provider/names.rs

+#[derive(PartialEq, Eq)] // VarULE wants these to be byte equality
+#[derive(Debug, VarULE)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize))]
+#[repr(transparent)]


thought: we really should see if we can get someone to make the make_wrapper_varule macro

Manishearth · 2023-06-07T07:27:59Z

components/timezone/src/provider/names.rs

+pub struct Bcp47ToIanaMapV1<'data> {
+    /// A map from BCP-47 time zone identifiers to IANA time zone identifiers
+    #[cfg_attr(feature = "serde", serde(borrow))]
+    pub map: ZeroMap<'data, UnvalidatedTinyAsciiStr<8>, str>,


thought: we could store the str unvalidated and lazily validate as well (GIGO returning None when validation is not possible)

unclear if this is at all worth it

Manishearth · 2023-06-07T07:29:33Z

components/timezone/src/provider.rs

@@ -21,6 +21,11 @@ use tinystr::TinyAsciiStr;
 use zerovec::ule::{AsULE, ULE};
 use zerovec::{ZeroMap2d, ZeroSlice, ZeroVec};

+pub mod names;


issue: not sure if we can say this fixes #2909 since this is just the data model, there's no fetcher struct yet

Just pushed the API

sffc · 2023-06-07T07:42:23Z

This PR took about 4 hours to write, and it gives me the IANA-to-BCP47 mappings in all supported programming languages. Not bad in the grand scheme of things, but it still feels like there is a ton of boilerplate and manual steps that we should be able to automate. The actual business logic is very centralized in just a small handful of functions.

Manishearth · 2023-06-07T09:33:09Z

ffi/diplomat/src/iana_bcp47_mapper.rs

+
+    impl ICU4XIanaToBcp47Mapper {
+        #[diplomat::rust_link(icu::timezone::IanaToBcp47Mapper::try_new_unstable, FnInStruct)]
+        pub fn create(


question: is it worth exposing the getter directly as well?

sffc · 2023-06-08T00:36:06Z

Note to self: The CI failure is Checking crate=icu_capi features=[icu_datetime]

robertbastian · 2023-06-08T11:08:16Z

components/timezone/src/provider/names.rs

+    }
+}
+
+impl NormalizedTimeZoneIdStr {


You have Box and & constructors, but don't we need Cow to keep postcard alloc-free while allowing JSON?

I guess because this is only used in a ZV, postcard uses ULE instead of Serialize, but this should still work correctly in case someone uses this in a non-ULE location.

More constructors can be added on an as-needed basis; I wanted to add the ones that are reachable. In fact I should probably double check if there are any I can delete.

/s/constructors/serde impls

robertbastian · 2023-06-08T11:13:52Z

components/timezone/src/provider/names.rs

+)]
+#[cfg_attr(feature = "serde", derive(serde::Deserialize))]
+#[yoke(prove_covariance_manually)]
+pub struct IanaToBcp47MapV1<'data> {


Having both IanaToBcp and BcpToIana seems wasteful, it's the same data in different order, and it's not small data. How about two ZeroMaps where the value is the index of the key in the other zeromap?

It's not exactly the same data; BCP47-to-IANA only contains canonical IDs, whereas IANA-to-BCP47 includes legacy and alias IANA IDs. Note that one is about 23% smaller than the other.

By far the primary use case is IANA-to-BCP47. Most clients don't need BCP47-to-IANA, but it may be needed to support things like the Temporal API. Everyone else can slice out the other 12 kB.

This follows the same design pattern as property names, which have separate keys for the two directions of mappings

Making it a single data struct would increase data size for the common case (but I'm not sure by how much). Your proposed data model works well with ZeroMap; but I was also thinking that I would like to move these to AsciiTrie which doesn't support random indexing.

Would you like to discuss further?

Will AsciiTrie be ready by the time is is released, or will that be V2 anyway?

I'll experiment with some different data models, including experimental AsciiTrie, and report back the findings.

Some data size findings:

iana2bcp ZeroMap: 14475B (this PR)

bcp2iana ZeroMap: 11249B (this PR)

Combined iana/bcp with dual ZeroMaps: 15521B (Rob's suggestion)

iana2bcp AsciiTrie: 9820B

I think a 7% size increase is worth paying for getting the other key for free. AsciiTrie will need a new data marker anyway.

Discussion: ok to go with AsciiTrie as long as it lands in time for 1.3

robertbastian

.

sffc · 2023-06-28T12:32:11Z

Blocked on #2722 landing first.

sffc · 2023-08-26T00:35:52Z

I'm planning to update this PR soon. I plan to remove the BCP-to-IANA mappings because they need to have a more careful design. CLDR and ECMA are both trying to agree on a standard mapping of canonical IANA aliases. We may end up wanting to return something more expressive such as an annotated set of multiple IANA names per BCP47 ID.

sffc · 2023-08-31T19:47:46Z

CC @hsivonen

Although this data structure is optimized for IANA-to-BCP47 mapping, it should be able to be used to in effect get the list of available time zones (#3970).

robertbastian · 2023-10-25T16:37:12Z

Superseded^

sffc added 6 commits June 6, 2023 22:17

Add boilerplate for NormalizedTimeZoneIdStr

1ae7862

Add data structs based on properties name structs

5dc8c6d

fmt

5341e17

Add IanaToBcp47MapV1Marker to datagen

85df3f9

Add Bcp47ToIanaMapV1Marker to datagen

2fc7d31

fmt

1e49656

sffc commented Jun 7, 2023

View reviewed changes

sffc marked this pull request as ready for review June 7, 2023 06:29

sffc requested review from robertbastian, Manishearth, nordzilla and a team as code owners June 7, 2023 06:29

sffc removed request for a team and robertbastian June 7, 2023 06:29

sffc added 4 commits June 6, 2023 23:52

Add IanaToBcp47Mapper

062a52b

Add Bcp47ToIanaMapper

e0964a7

Use IanaToBcp47Mapper in docs tests

f3d5859

Add FFI for the new Rust functions

dceaa5b

Manishearth approved these changes Jun 7, 2023

View reviewed changes

Manishearth previously approved these changes Jun 7, 2023

View reviewed changes

Add C++ test

744d2d5

sffc dismissed Manishearth’s stale review via 744d2d5 June 7, 2023 07:39

sffc requested review from zbraniecki and gregtatum as code owners June 7, 2023 07:39

sffc requested review from Manishearth and removed request for zbraniecki and gregtatum June 7, 2023 07:42

Generate-readmes

a58a33a

Manishearth previously approved these changes Jun 7, 2023

View reviewed changes

Fix icu_capi cargo-all-features

ad9e056

sffc dismissed Manishearth’s stale review via ad9e056 June 8, 2023 01:07

Manishearth approved these changes Jun 8, 2023

View reviewed changes

robertbastian reviewed Jun 8, 2023

View reviewed changes

sffc requested a review from robertbastian June 12, 2023 23:57

robertbastian reviewed Jun 13, 2023

View reviewed changes

sffc mentioned this pull request Aug 21, 2023

Transliterator IDs with unknown BCP47 IDs #3891

Closed

sffc mentioned this pull request Sep 11, 2023

Add IANA-to-BCP47 and reverse mappings #4024

Merged

robertbastian closed this Oct 25, 2023

sffc deleted the timezone-name-lookup branch October 25, 2023 16:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add IANA/BCP47 time zone name mappings #3499

Add IANA/BCP47 time zone name mappings #3499

sffc commented Jun 7, 2023 •

edited

Loading

sffc Jun 7, 2023

Manishearth Jun 7, 2023

Manishearth Jun 7, 2023

Manishearth Jun 7, 2023

sffc Jun 7, 2023

sffc commented Jun 7, 2023

Manishearth Jun 7, 2023

sffc commented Jun 8, 2023

robertbastian Jun 8, 2023

sffc Jun 8, 2023

robertbastian Jun 8, 2023

robertbastian Jun 8, 2023

sffc Jun 8, 2023

robertbastian Jun 8, 2023

sffc Jun 8, 2023

sffc Jun 8, 2023

robertbastian Jun 13, 2023

sffc Jun 13, 2023

robertbastian left a comment

sffc commented Jun 28, 2023

sffc commented Aug 26, 2023

sffc commented Aug 31, 2023

robertbastian commented Oct 25, 2023

Add IANA/BCP47 time zone name mappings #3499

Add IANA/BCP47 time zone name mappings #3499

Conversation

sffc commented Jun 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sffc commented Jun 7, 2023

Choose a reason for hiding this comment

sffc commented Jun 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robertbastian left a comment

Choose a reason for hiding this comment

sffc commented Jun 28, 2023

sffc commented Aug 26, 2023

sffc commented Aug 31, 2023

robertbastian commented Oct 25, 2023

sffc commented Jun 7, 2023 •

edited

Loading