CodePointTrie data provider #1167

echeran · 2021-10-11T22:47:51Z

This PR is rebased on top of PR #1161, which addresses the upstream dependency issue #1072 -- designing the data provider struct for CodePointTrie data. This PR will implement the actual code needed for the data provider, based on that design.

The pieces of work involved, to wit:

(imho) Renaming of data provider structs for consistency Refactor properties to separate crate #1153
Data provider code for CPT provider
Transformer code between source data struct (enumerated property TOML file) and data provider struct (UnicodePropertyMapV1)

TrieType no longer exists, so we don't need an awkward name for TrieTypeEnum.

…or CodePointTrie data

…r_uprops

echeran · 2021-10-11T22:48:10Z

cc @hsivonen

jira-pull-request-webhook · 2021-10-12T18:47:17Z

Notice: the branch changed across the force-push!

provider/uprops/src/enum_codepointtrie.rs is different
utils/codepointtrie/src/codepointtrie.rs is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

provider/uprops/src/enum_codepointtrie.rs

sffc

Mostly LGTM! Please update to main to remove the obsolete diffs from Iain's work.

sffc · 2021-10-18T22:13:51Z

components/uniset/src/enum_props.rs

+}
+
+impl From<&TinyStr16> for EnumeratedProperty {
+    fn from(prop_short_alias: &TinyStr16) -> Self {


Do this one of two ways:

Match on a &str rather than a &TinyStr16

Match on a TinyStr16 by value as described in Recommendation for pattern matching zbraniecki/tinystr#22

Also, please make sure we are actually using this. If you can delete it, please do, because when we actually implement string-to-property parsing, it should be data-driven, not hard coded in this impl.

Please fix.

provider/uprops/src/enum_codepointtrie.rs

utils/codepointtrie/src/codepointtrie.rs

provider/uprops/src/enum_codepointtrie.rs

utils/codepointtrie/Cargo.toml

provider/uprops/src/enum_codepointtrie.rs

utils/codepointtrie/src/codepointtrie.rs

… separate crate)

iainireland · 2021-10-19T22:46:49Z

provider/uprops/src/provider.rs

@@ -1,51 +0,0 @@
-// This file is part of ICU4X. For terms of use, please see the file


Wait, why is this file being deleted? I'm using this code in icu4x_js_regexp.

We could rename it to ~PropertiesUnicodeSetDataProvider if necessary.

That was from a few days ago when I didn't recognize how it was being used. I recognized it since then and have it un-deleted locally. Let me push up the local changes.

utils/codepointtrie/src/codepointtrie.rs

sffc · 2021-10-20T02:06:39Z

utils/codepointtrie/Cargo.toml

@@ -5,7 +5,7 @@
 [package]


Please use this PR to move the content from https://github.com/unicode-org/icu4x/blob/main/utils/codepointtrie/src/provider.rs into components/properties/src/provider.rs, and also remove the dependency on the data provider from utils/codepointtrie.

First part done (contents of codepointtrie provider.rs -> properties provider.rs).

CodePointTrie struct in codepointtrie.rs has default impls of Yokeable and ZeroCopyFrom implemented, so I didn't think I could remove the dependency on icu_provider within icu_codepointtrie.

Please change the dependency to be yoke instead of icu_provider

sffc · 2021-10-20T02:22:31Z

components/properties/src/provider.rs

+
+        // ResourceKey subcategory string is the short alias of the property
+
+        (GENERAL_CATEGORY_V1, "gc"),


Discussion: By using the same macro here, we are putting these in the same namespace as the binary properties. For example: "uniset/alnum@1" and "uniset/gc@1". Do we want a separate namespace like "prop_maps/gc@1" ? We could defer this decision until later (but before v1).

components/properties/src/trievalue.rs

provider/uprops/src/enum_codepointtrie.rs

utils/codepointtrie/src/codepointtrie.rs

…u_properties

sffc

The diffs from last time look good; I'll do another pass soon

…nttrie

iainireland and others added 7 commits October 6, 2021 17:06

Rename TrieTypeEnum to TrieType

d1ca58c

TrieType no longer exists, so we don't need an awkward name for TrieTypeEnum.

Implement Yokeable/ZeroCopyFrom for CodePointTrie and data struct

2731299

Cargo fmt + minor fixes

3519819

Add CPT struct to icu_provider_uprops data source struct

9142762

Renames data providers for UnicodeSet data ahead of introducing one f…

7ce722e

…or CodePointTrie data

Matches CPT version to project/sub-crates, adds CPT as dep to provide…

7784c1d

…r_uprops

Add WIP code for data provider for CodePointTrie data

1f6d7e7

echeran requested a review from iainireland October 11, 2021 22:47

echeran and others added 2 commits October 11, 2021 18:46

More WIP code for CodePointTrie data provider implementation

3a890f5

Fix error

888edc5

Manishearth force-pushed the cpt-data-transformer branch from b468891 to 888edc5 Compare October 12, 2021 18:47

Manishearth mentioned this pull request Oct 12, 2021

Add FromIterator impl for ZeroVec #1169

Merged

Manishearth reviewed Oct 12, 2021

View reviewed changes

provider/uprops/src/enum_codepointtrie.rs Outdated Show resolved Hide resolved

echeran added 2 commits October 12, 2021 13:39

Merge branch 'main' into cpt-data-transformer

4b6c986

Simplify constructing ZeroVec using ZV's new FromIterator impl

1186512

echeran mentioned this pull request Oct 13, 2021

Refactor properties to separate crate #1153

Merged

echeran requested a review from sffc October 18, 2021 21:49

sffc requested changes Oct 18, 2021

View reviewed changes

Manishearth reviewed Oct 19, 2021

View reviewed changes

provider/uprops/src/enum_codepointtrie.rs Show resolved Hide resolved

utils/codepointtrie/src/codepointtrie.rs Outdated Show resolved Hide resolved

utils/codepointtrie/src/codepointtrie.rs Outdated Show resolved Hide resolved

echeran added 2 commits October 19, 2021 13:51

Merge branch 'main' into cpt-data-transformer

b67d033

Merge current snapshot of PR unicode-org#1153 (refactor properties to…

4abb8a4

… separate crate)

iainireland reviewed Oct 19, 2021

View reviewed changes

echeran added 3 commits October 19, 2021 16:16

Update path to uniset crate in CI job for benchmarking

ee4e8e9

Merge branch 'main' into cpt-data-transformer

69fc6e1

Implement TrieValue for GeneralSubcategory

c543200

sffc reviewed Oct 20, 2021

View reviewed changes

echeran requested a review from sffc October 20, 2021 02:09

sffc mentioned this pull request Oct 20, 2021

Set CPT ValueWidth::DATA_GET_ERROR_VALUE to 0 #1183

Closed

sffc reviewed Oct 20, 2021

View reviewed changes

echeran added 10 commits October 20, 2021 14:04

Implement TrieValue for Script

40381be

Rename TrieValue trait's associate type for Result errors

d4bbcbd

Remove unneeded dependency

997db93

Revert version number of icu_codepointtrie

28b51c0

Move data structs for UnicodePropertyMap from icu_codepointtrie to ic…

e10eb66

…u_properties

Error message rewording

5b32a4d

Finish reverting unneeded renaming/refactoring in icu_properties

499b817

Add docstrings for the uprops data providers

7139775

Add test for Script using data provider for CodePointTrie data

fd0f748

Export CPT data provider symbol publicly

4ce245c

echeran requested a review from sffc October 20, 2021 21:47

Merge branch 'main' into cpt-data-transformer

b8aada4

sffc reviewed Oct 20, 2021

View reviewed changes

echeran added 2 commits October 20, 2021 16:10

Declare no_std for icu_codepointtrie

8aca45a

Add extern crate... to import alloc libs

a39dd49

echeran marked this pull request as ready for review October 21, 2021 00:00

echeran requested a review from a team as a code owner October 21, 2021 00:00

echeran added 2 commits October 20, 2021 17:13

Remove unused custom code for string -> enum conversion

3689a11

Replace icu_provider dep with yoke, remove std feature in icu_codepoi…

f24108d

…nttrie

sffc previously approved these changes Oct 21, 2021

View reviewed changes

echeran mentioned this pull request Oct 21, 2021

Discuss data key category for properties resources #1196

Closed

3 tasks

Add derive feature to yoke dependency in icu_codepointtrie

0a32faf

echeran dismissed sffc’s stale review via 0a32faf October 21, 2021 00:42

sffc approved these changes Oct 21, 2021

View reviewed changes

echeran merged commit 3ea4f80 into unicode-org:main Oct 21, 2021

echeran deleted the cpt-data-transformer branch October 21, 2021 01:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CodePointTrie data provider #1167

CodePointTrie data provider #1167

echeran commented Oct 11, 2021 •

edited

Loading

echeran commented Oct 11, 2021

jira-pull-request-webhook bot commented Oct 12, 2021

sffc left a comment

sffc Oct 18, 2021

sffc Oct 18, 2021

sffc Oct 21, 2021

iainireland Oct 19, 2021

echeran Oct 19, 2021

sffc Oct 20, 2021

echeran Oct 20, 2021

sffc Oct 20, 2021

sffc Oct 20, 2021

sffc left a comment

		@@ -1,51 +0,0 @@
		// This file is part of ICU4X. For terms of use, please see the file


		// ResourceKey subcategory string is the short alias of the property

		(GENERAL_CATEGORY_V1, "gc"),

CodePointTrie data provider #1167

CodePointTrie data provider #1167

Conversation

echeran commented Oct 11, 2021 • edited Loading

echeran commented Oct 11, 2021

jira-pull-request-webhook bot commented Oct 12, 2021

sffc left a 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

Choose a reason for hiding this comment

sffc left a comment

Choose a reason for hiding this comment

echeran commented Oct 11, 2021 •

edited

Loading