Skip to content

Number format in ICU4X: add known issues and unsupported rounding modes #470

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

Merged
merged 4 commits into from
Jun 27, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 34 additions & 4 deletions executors/rust/src/numberfmt.rs
Original file line number Diff line number Diff line change
@@ -2,10 +2,10 @@

#[cfg(any(ver = "1.3", ver = "1.4", ver = "1.5", ver = "2.0-beta1"))]
use fixed_decimal::FixedDecimal as Decimal;
#[cfg(ver = "2.0-beta1")]
use fixed_decimal::RoundingMode;
#[cfg(not(any(ver = "1.3", ver = "1.4", ver = "1.5", ver = "2.0-beta1")))]
use fixed_decimal::{Decimal, SignedRoundingMode, UnsignedRoundingMode};
use fixed_decimal::{Decimal, RoundingIncrement, SignedRoundingMode, UnsignedRoundingMode};
#[cfg(ver = "2.0-beta1")]
use fixed_decimal::{RoundingIncrement, RoundingMode};

use fixed_decimal::SignDisplay;
// TODO: use fixed_decimal::ScientificDecimal;
@@ -56,6 +56,7 @@ struct NumberFormatOptions {
notation: Option<String>,
numbering_system: Option<String>,
rounding_mode: Option<String>,
rounding_increment: Option<isize>,
sign_display: Option<String>,
style: Option<String>,
unit: Option<String>,
@@ -206,7 +207,7 @@ pub fn run_numberformat_test(json_obj: &Value) -> Result<Value, String> {
_ => input_num.half_even(-(x as i16)),
};
#[cfg(not(any(ver = "1.3", ver = "1.4", ver = "1.5")))]
input_num.round_with_mode(
input_num.round_with_mode_and_increment(
-(x as i16),
#[cfg(any(ver = "1.3", ver = "1.4", ver = "1.5", ver = "2.0-beta1"))]
match option_struct.rounding_mode.as_deref() {
@@ -219,6 +220,14 @@ pub fn run_numberformat_test(json_obj: &Value) -> Result<Value, String> {
Some("halfExpand") => RoundingMode::HalfExpand,
Some("halfTrunc") => RoundingMode::HalfTrunc,
Some("halfEven") => RoundingMode::HalfEven,
Some("halfOdd") | Some("unnecessary") => {
return Ok(json!({
"label": label,
"error_detail": option_struct.rounding_mode,
"unsupported": "roundingMode",
"error_type": "unsupported",
}));
}
_ => RoundingMode::HalfEven,
},
#[cfg(not(any(ver = "1.3", ver = "1.4", ver = "1.5", ver = "2.0-beta1")))]
@@ -238,8 +247,29 @@ pub fn run_numberformat_test(json_obj: &Value) -> Result<Value, String> {
Some("halfEven") => {
SignedRoundingMode::Unsigned(UnsignedRoundingMode::HalfEven)
}
Some("halfOdd") | Some("unnecessary") => {
return Ok(json!({
"label": label,
"error_detail": option_struct.rounding_mode,
"unsupported": "roundingMode",
"error_type": "unsupported",
}));
}
_ => SignedRoundingMode::Unsigned(UnsignedRoundingMode::HalfEven),
},
match option_struct.rounding_increment.as_ref() {
Some(2) => RoundingIncrement::MultiplesOf2,
Some(5) => RoundingIncrement::MultiplesOf5,
Some(25) => RoundingIncrement::MultiplesOf25,
Some(1) | None => RoundingIncrement::MultiplesOf1,
_ => {
return Ok(json!({
"label": label,
"error_detail": option_struct.rounding_increment,
"error_type": "bad rounding increment",
}));
}
},
);
input_num.trim_end();
}
36 changes: 25 additions & 11 deletions verifier/check_known_issues.py
Original file line number Diff line number Diff line change
@@ -64,7 +64,8 @@ class knownIssueType(Enum):
langnames_bracket_parens = 'brackets_vs_parentheses'

# Number format
# Support expected errors https://github.com/unicode-org/conformance/issues/242
# https://github.com/unicode-org/icu4x/issues/6678
number_fmt_icu4x_small_fractional_numbers = 'icu4x#6678 small fractional numbers'
number_fmt_inexact_rounding = 'Rounding unnecessary'

# Plural rules
@@ -303,18 +304,30 @@ def langname_brackets(test):
# Number format known issues
def check_number_fmt_issues(test, platform_info):
input_data = test['input_data']
if 'expected' in test:
expected = test['expected']
if expected == 'Inexact' and input_data['options']['roundingMode'] == 'unnecessary':
return knownIssueType.number_fmt_inexact_rounding
result = test.get('result', None)
expected = test.get('expected', None)
options = input_data.get('options', None)

if 'result' not in test:
# This must be an error
if not result:
# This must be an error because no result is found
if 'error' in test and re.match(r'Rounding is required', test['error']):
return knownIssueType.number_fmt_inexact_rounding
# No known issue for this case
return None

try:
if platform_info['platform'] == 'ICU4X' and result:
if options:
notation = options.get('notation', None)
input_value = input_data.get('input', None)
if notation == 'compact' and result[0:1] == '-' and abs(float(input_value)) < 1.0:
return knownIssueType.number_fmt_icu4x_small_fractional_numbers

if expected == 'Inexact' and 'roundingMode' in input_data['options']:
if input_data['options']['roundingMode'] == 'unnecessary':
return knownIssueType.number_fmt_inexact_rounding

except BaseException as error:
pass
return None

def check_plural_rules_issues(test):
try:
@@ -345,10 +358,11 @@ def compute_known_issues_for_single_test(test_type, test, platform_info):
known_issue_found = check_likely_subtags_issues(test)
elif test_type == ddt_data.testType.lang_names.value:
known_issue_found = check_langnames_issues(test)
elif test_type == ddt_data.testType.plural_rules.value:
known_issue_found = check_plural_rules_issues(test)
elif test_type == ddt_data.testType.number_fmt.value:
known_issue_found = check_number_fmt_issues(test, platform_info)
elif test_type == ddt_data.testType.plural_rules.value:
known_issue_found = check_plural_rules_issues(test)

# TODO: Add checks here for known issues in other test types

return known_issue_found
6 changes: 3 additions & 3 deletions verifier/testreport.py
Original file line number Diff line number Diff line change
@@ -402,14 +402,14 @@ def create_html_report(self):
except KeyError:
self.platform_info['icuVersion'] = 'Unknown'

platform_info = '%s %s - ICU %s' % (
platform_data = '%s %s - ICU %s' % (
self.platform_info['platform'], self.platform_info['platformVersion'],
self.platform_info['icuVersion'])
html_map = {'test_type': self.test_type,
'exec': self.exec,
# TODO: Change to 'icu4x' instead of rust
'library_name': self.library_name,
'platform_info': platform_info,
'platform_info': platform_data,
'test_environment': dict_to_html(self.test_environment),
'timestamp': self.timestamp,
'total_tests': self.number_tests,
@@ -442,7 +442,7 @@ def create_html_report(self):
new_known_issues = check_issues(
self.test_type,
[self.failing_tests, self.test_errors, self.unsupported_cases],
platform_info)
self.platform_info)

if new_known_issues:
self.known_issues.extend(new_known_issues)
1 change: 0 additions & 1 deletion verifier/verifier.py
Original file line number Diff line number Diff line change
@@ -236,7 +236,6 @@ def parallel_verify_data_results(self):
except BaseException as err:
logging.error('%s: Problem with verify_data for %s',
err, verify_plans)

return result
else:
logging.info('Running serially!')