ICU-21451 Implement inverse unit handling for new consumption unit preferences in CLDR

See #1550
This commit is contained in:
Hugo van der Merwe 2021-02-02 15:09:25 +00:00
parent 5dc0513b29
commit 8e69552228
13 changed files with 90 additions and 62 deletions

View file

@ -925,6 +925,16 @@ units:table(nofallback){
unit{"liter-per-kilometer"}
}
}
CA{
{
unit{"mile-per-gallon-imperial"}
}
}
GB{
{
unit{"mile-per-gallon-imperial"}
}
}
IT{
{
unit{"liter-per-kilometer"}
@ -965,32 +975,6 @@ units:table(nofallback){
unit{"liter-per-kilometer"}
}
}
}
}
"consumption-inverse"{
"default"{
001{
{
unit{"kilometer-per-centiliter"}
}
}
}
"vehicle-fuel"{
001{
{
unit{"kilometer-per-centiliter"}
}
}
CA{
{
unit{"mile-per-gallon-imperial"}
}
}
GB{
{
unit{"mile-per-gallon-imperial"}
}
}
US{
{
unit{"mile-per-gallon"}
@ -1328,10 +1312,12 @@ units:table(nofallback){
unit{"meter"}
}
{
geq{"10"}
skeleton{"precision-increment/10"}
unit{"meter"}
}
{
skeleton{"precision-increment/1"}
unit{"meter"}
}
}
@ -1346,6 +1332,12 @@ units:table(nofallback){
unit{"yard"}
}
{
geq{"10"}
skeleton{"precision-increment/10"}
unit{"yard"}
}
{
skeleton{"precision-increment/1"}
unit{"yard"}
}
}
@ -1362,9 +1354,14 @@ units:table(nofallback){
unit{"meter"}
}
{
geq{"10"}
skeleton{"precision-increment/10"}
unit{"meter"}
}
{
skeleton{"precision-increment/1"}
unit{"meter"}
}
}
US{
{
@ -1377,9 +1374,14 @@ units:table(nofallback){
unit{"foot"}
}
{
geq{"10"}
skeleton{"precision-increment/10"}
unit{"foot"}
}
{
skeleton{"precision-increment/1"}
unit{"foot"}
}
}
}
"snowfall"{

View file

@ -282,6 +282,10 @@ int32_t getPreferenceMetadataIndex(const MaybeStackVector<UnitPreferenceMetadata
if (U_FAILURE(status)) { return -1; }
if (idx >= 0) { return idx; }
if (!foundCategory) {
// TODO: failures can happen if units::getUnitCategory returns a category
// that does not appear in unitPreferenceData. Do we want a unit test that
// checks unitPreferenceData has full coverage of categories? Or just trust
// CLDR?
status = U_ILLEGAL_ARGUMENT_ERROR;
return -1;
}
@ -370,12 +374,12 @@ CharString U_I18N_API getUnitCategory(const char *baseUnitIdentifier, UErrorCode
const UChar *uCategory =
ures_getStringByKey(unitQuantities.getAlias(), baseUnitIdentifier, &categoryLength, &status);
if (U_FAILURE(status)) {
// TODO(CLDR-13787,hugovdm): special-casing the consumption-inverse
// case. Once CLDR-13787 is clarified, this should be generalised (or
// possibly removed):
// TODO(icu-units#130): support inverting any unit, with correct
// fallback logic: inversion and fallback may depend on presence or
// absence of a usage for that category.
if (uprv_strcmp(baseUnitIdentifier, "meter-per-cubic-meter") == 0) {
status = U_ZERO_ERROR;
result.append("consumption-inverse", status);
result.append("consumption", status);
return result;
}
}
@ -415,7 +419,11 @@ void U_I18N_API UnitPreferences::getPreferencesFor(StringPiece category, StringP
const UnitPreference *const *&outPreferences,
int32_t &preferenceCount, UErrorCode &status) const {
int32_t idx = getPreferenceMetadataIndex(&metadata_, category, usage, region, status);
if (U_FAILURE(status)) { return; }
if (U_FAILURE(status)) {
outPreferences = nullptr;
preferenceCount = 0;
return;
}
U_ASSERT(idx >= 0); // Failures should have been taken care of by `status`.
const UnitPreferenceMetadata *m = metadata_[idx];
outPreferences = unitPrefs_.getAlias() + m->prefsOffset;

View file

@ -56,7 +56,7 @@ UnitsRouter::UnitsRouter(MeasureUnit inputUnit, StringPiece region, StringPiece
CharString category = getUnitCategory(baseUnit.getIdentifier(), status);
const UnitPreference *const *unitPreferences;
int32_t preferencesCount;
int32_t preferencesCount = 0;
prefs.getPreferencesFor(category.data(), usage, region, unitPreferences, preferencesCount, status);
for (int i = 0; i < preferencesCount; ++i) {

View file

@ -1239,7 +1239,7 @@ void NumberFormatterApiTest::unitUsage() {
u"8,8 km",
u"900 m",
u"90 m",
u"10 m",
u"9 m",
u"0 m");
uTestCase = u"unitUsage() en-GB road";
@ -1274,8 +1274,8 @@ void NumberFormatterApiTest::unitUsage() {
u"54 mi",
u"5.4 mi",
u"0.54 mi",
u"96 yd",
u"9.6 yd",
u"100 yd",
u"10 yd",
u"0 yd");
uTestCase = u"unitUsage() en-US road";

View file

@ -10,6 +10,7 @@
using namespace ::icu::units;
// These test are no in ICU4J. TODO: consider porting them to Java?
class UnitsDataTest : public IntlTest {
public:
UnitsDataTest() {}
@ -38,12 +39,14 @@ void UnitsDataTest::testGetUnitCategory() {
const char *expectedCategory;
} testCases[]{
{"kilogram-per-cubic-meter", "mass-density"},
{"cubic-meter-per-kilogram", "specific-volume"},
{"meter-per-second", "speed"},
// TODO(icu-units#130): inverse-speed
// {"second-per-meter", "speed"},
// Consumption specifically supports inverse units (mile-per-galon,
// liter-per-100-kilometer):
{"cubic-meter-per-meter", "consumption"},
// TODO(CLDR-13787,hugovdm): currently we're treating
// consumption-inverse as a separate category. Once consumption
// preference handling has been clarified by CLDR-13787, this function
// should be fixed.
{"meter-per-cubic-meter", "consumption-inverse"},
{"meter-per-cubic-meter", "consumption"},
};
IcuTestErrorCode status(*this, "testGetUnitCategory");

View file

@ -83,14 +83,22 @@ consumption; vehicle-fuel; BR; 11 / 10000000; 1.1E-6; cubic-meter-per-meter; 11
consumption; vehicle-fuel; BR; 1 / 1000000; 1.0E-6; cubic-meter-per-meter; 1; 1.0; liter-per-kilometer
consumption; vehicle-fuel; BR; 9 / 10000000; 9.0E-7; cubic-meter-per-meter; 9 / 10; 0.9; liter-per-kilometer
consumption-inverse; default; 001; 110000000; 1.1E8; meter-per-cubic-meter; 11 / 10; 1.1; kilometer-per-centiliter
consumption-inverse; default; 001; 100000000; 1.0E8; meter-per-cubic-meter; 1; 1.0; kilometer-per-centiliter
consumption-inverse; default; 001; 90000000; 9.0E7; meter-per-cubic-meter; 9 / 10; 0.9; kilometer-per-centiliter
# TODO: these are local ICU-specific edits until the CLDR test file is updated:
# consumption-inverse; default; 001; 110000000; 1.1E8; meter-per-cubic-meter; 11 / 10; 1.1; kilometer-per-centiliter
consumption; default; 001; 110000000; 1.1E8; meter-per-cubic-meter; 10 / 11; 0.90909090909090906; liter-per-100-kilometer
# consumption-inverse; default; 001; 100000000; 1.0E8; meter-per-cubic-meter; 1; 1.0; kilometer-per-centiliter
consumption; default; 001; 100000000; 1.0E8; meter-per-cubic-meter; 1; 1.0; liter-per-100-kilometer
# consumption-inverse; default; 001; 90000000; 9.0E7; meter-per-cubic-meter; 9 / 10; 0.9; kilometer-per-centiliter
consumption; default; 001; 90000000; 9.0E7; meter-per-cubic-meter; 10 / 9; 1.1111111111111112; liter-per-100-kilometer
consumption-inverse; vehicle-fuel; 001; 110000000; 1.1E8; meter-per-cubic-meter; 11 / 10; 1.1; kilometer-per-centiliter
consumption-inverse; vehicle-fuel; 001; 100000000; 1.0E8; meter-per-cubic-meter; 1; 1.0; kilometer-per-centiliter
consumption-inverse; vehicle-fuel; 001; 90000000; 9.0E7; meter-per-cubic-meter; 9 / 10; 0.9; kilometer-per-centiliter
# consumption-inverse; vehicle-fuel; 001; 110000000; 1.1E8; meter-per-cubic-meter; 11 / 10; 1.1; kilometer-per-centiliter
consumption; vehicle-fuel; 001; 110000000; 1.1E8; meter-per-cubic-meter; 10 / 11; 0.90909090909090906; liter-per-100-kilometer
# consumption-inverse; vehicle-fuel; 001; 100000000; 1.0E8; meter-per-cubic-meter; 1; 1.0; kilometer-per-centiliter
consumption; vehicle-fuel; 001; 100000000; 1.0E8; meter-per-cubic-meter; 1; 1.0; liter-per-100-kilometer
# consumption-inverse; vehicle-fuel; 001; 90000000; 9.0E7; meter-per-cubic-meter; 9 / 10; 0.9; kilometer-per-centiliter
consumption; vehicle-fuel; 001; 90000000; 9.0E7; meter-per-cubic-meter; 10 / 9; 1.1111111111111112; liter-per-100-kilometer
# TODO: these still pass, as expected/desired. Leaving them categorised as consumption-inverse, this is ignored by tests anyway
consumption-inverse; vehicle-fuel; US; 52800000000 / 112903; 467658.0781732992; meter-per-cubic-meter; 11 / 10; 1.1; mile-per-gallon
consumption-inverse; vehicle-fuel; US; 48000000000 / 112903; 425143.707430272; meter-per-cubic-meter; 1; 1.0; mile-per-gallon
consumption-inverse; vehicle-fuel; US; 43200000000 / 112903; 382629.3366872448; meter-per-cubic-meter; 9 / 10; 0.9; mile-per-gallon

View file

@ -64,7 +64,9 @@ public class UnitPreferences {
result = getUnitPreferences(category, subUsage, region);
if (result != null) break;
}
// TODO: if a category is missing, we get an assertion failure, or we
// return null, causing a NullPointerException. In C++, we return an
// U_MISSING_RESOURCE_ERROR error.
assert (result != null) : "At least the category must be exist";
return result;
}

View file

@ -64,11 +64,10 @@ public class UnitsData {
String baseUnitIdentifier = MeasureUnit.fromMeasureUnitImpl(baseMeasureUnit).getIdentifier();
if (baseUnitIdentifier.equals("meter-per-cubic-meter")) {
// TODO(CLDR-13787,hugovdm): special-casing the consumption-inverse
// case. Once CLDR-13787 is clarified, this should be generalised (or
// possibly removed):
return "consumption-inverse";
// TODO(icu-units#130): support inverting any unit, with correct
// fallback logic: inversion and fallback may depend on presence or
// absence of a usage for that category.
return "consumption";
}
return this.categories.mapFromUnitToCategory.get(baseUnitIdentifier);

View file

@ -1,3 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:a4adc39da0522d36906b035fe47aa7a9becfefaa4a5ccef2cdf6f5c23d194777
size 13306768
oid sha256:e738e530bcd2dcafff1de1d603c79d5a1edc04c095ca52366259c354f19e56ed
size 13306751

View file

@ -1,3 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:4dce4d778b403e5b91fbe11eba9e0b342c9fe28f2c8d39c7ae8641b311c9a71f
size 95083
oid sha256:19f02ee2a2dc722a729fa9258175a738fc6021d252769b85c023a927135c7c26
size 95080

View file

@ -1,3 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:c80f58afc2714a7e1af4706c075a23ae37de228f0d383bf0cbaec340ade76030
size 726478
oid sha256:1970fbcc18ec8a8b86702fe73ffbba842e9379bd973edbfb4e189ac6ac6d2a83
size 723496

View file

@ -432,6 +432,7 @@ public class UnitsTest {
public void testUnitPreferencesWithCLDRTests() throws IOException {
class TestCase {
// TODO: content of outputUnitInOrder isn't checked? Only size?
final ArrayList<Pair<String, MeasureUnitImpl>> outputUnitInOrder = new ArrayList<>();
final ArrayList<BigDecimal> expectedInOrder = new ArrayList<>();
/**
@ -486,6 +487,11 @@ public class UnitsTest {
expectedInOrder.add(new BigDecimal(output.second));
}
}
public String toString() {
return "TestCase: " + category + ", " + usage + ", " + region +
"; Input: " + input + " " + inputUnit.first + "; Expected Values: " + expectedInOrder;
}
}
// Read Test data from the unitPreferencesTest
@ -517,7 +523,7 @@ public class UnitsTest {
.compareTwoBigDecimal(testCase.expectedInOrder.get(i),
BigDecimal.valueOf(measures.get(i).getNumber().doubleValue()),
BigDecimal.valueOf(0.00001))) {
fail(testCase.toString() + measures.toString());
fail("Test failed: " + testCase + "; Got unexpected result: " + measures);
}
}
}

View file

@ -1218,7 +1218,7 @@ public class NumberFormatterApiTest extends TestFmwk {
"8,8 km",
"900 m",
"90 m",
"10 m",
"9 m",
"0 m");
uTestCase = "unitUsage() en-GB road";
@ -1250,8 +1250,8 @@ public class NumberFormatterApiTest extends TestFmwk {
"54 mi",
"5.4 mi",
"0.54 mi",
"96 yd",
"9.6 yd",
"100 yd",
"10 yd",
"0 yd");
uTestCase = "unitUsage() en-US road";