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

Data and provider for compact decimal formatting #2883

Merged
merged 63 commits into from
Dec 16, 2022

Conversation

eggrobin
Copy link
Member

@eggrobin eggrobin commented Dec 13, 2022

Towards #1267.

@CLAassistant
Copy link

CLAassistant commented Dec 13, 2022

CLA assistant check
All committers have signed the CLA.

@eggrobin eggrobin force-pushed the compact-decimal-formatting branch from c51d259 to 22097a2 Compare December 13, 2022 15:19
@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

@eggrobin eggrobin requested a review from sffc December 13, 2022 16:24
@eggrobin eggrobin marked this pull request as ready for review December 13, 2022 16:25
@eggrobin
Copy link
Member Author

CI fails because of Swahili. Supporting ; etc. would make things very messy, but that pattern should really be simplified to elfu 0, with the minus sign coming from the significand.

WARN  [icu_provider::error] ICU4X data error: Could not create compact decimal patterns: Multiple placeholders in compact decimal pattern elfu 0;elfu -0
WARN  [icu_provider::error] ICU4X data error: Could not create compact decimal patterns: latn
WARN  [icu_provider::error] ICU4X data error: Could not create compact decimal patterns (key: compactdecimal/long@1, request: sw)

@robertbastian
Copy link
Member

We had a similar issue in the list formatter, we fixed it in CLDR and added a special case in datagen.

@Manishearth
Copy link
Member

Yes, for both this and the scientific stuff we should just add some fixup hacks in datagen until cldr gets fixed

@eggrobin eggrobin requested review from Manishearth and sffc December 16, 2022 14:00
sffc
sffc previously approved these changes Dec 16, 2022
@sffc
Copy link
Member

sffc commented Dec 16, 2022

Please merge in main so that you can use iter1_copied

@eggrobin
Copy link
Member Author

Please merge in main so that you can use iter1_copied

I did, and I cannot, see #2897 (comment); reinstated into_iter1 on @Manishearth’s advice.

@eggrobin eggrobin requested a review from sffc December 16, 2022 16:01
@codecov-commenter
Copy link

Codecov Report

Merging #2883 (061953d) into main (346d1a2) will increase coverage by 0.12%.
The diff coverage is 82.28%.

@@            Coverage Diff             @@
##             main    #2883      +/-   ##
==========================================
+ Coverage   73.66%   73.78%   +0.12%     
==========================================
  Files         458      464       +6     
  Lines       39185    39596     +411     
==========================================
+ Hits        28866    29217     +351     
- Misses      10319    10379      +60     
Impacted Files Coverage Δ
experimental/compactdecimal/src/error.rs 0.00% <0.00%> (ø)
provider/datagen/src/registry.rs 40.00% <ø> (ø)
provider/datagen/src/transform/cldr/decimal/mod.rs 86.00% <ø> (+5.60%) ⬆️
provider/testdata/data/baked/any.rs 6.31% <ø> (ø)
provider/testdata/data/baked/mod.rs 78.03% <ø> (ø)
experimental/compactdecimal/src/provider.rs 55.00% <55.00%> (ø)
...r/datagen/src/transform/cldr/cldr_serde/numbers.rs 62.33% <58.06%> (-2.88%) ⬇️
...ider/datagen/src/transform/cldr/decimal/compact.rs 74.72% <74.72%> (ø)
...ider/datagen/src/transform/cldr/decimal/symbols.rs 76.36% <76.36%> (ø)
.../transform/cldr/decimal/compact_decimal_pattern.rs 96.68% <96.68%> (ø)
... and 4 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@sffc sffc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but let's make another PR to remove into_iter1 again

@sffc
Copy link
Member

sffc commented Dec 16, 2022

LGTM but let's make another PR to remove into_iter1 again

Never-mind: All OK

@eggrobin eggrobin merged commit 356bfa8 into unicode-org:main Dec 16, 2022
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

Successfully merging this pull request may close these issues.

8 participants