organicmaps/icu
13
Fork 0
mirror of https://github.com/unicode-org/icu.git synced 2025-04-10 15:42:14 +00:00
Code Issues 1 Projects Releases Wiki Activity Actions 1

ICU-20568 Improve negative measure handling for mixed units

Browse source 
See #1379
This commit is contained in:
Hugo van der Merwe 2020-09-30 16:31:20 +00:00
parent 75e7e0bb08
commit 9cb611d09f
8 changed files with 120 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

14
icu4c/source/i18n/number_longnames.cpp
View file

@ -455,6 +455,8 @@ const Modifier *MixedUnitLongNameHandler::getMixedUnitModifier(DecimalQuantity &
status = U_UNSUPPORTED_ERROR;
return &micros.helpers.emptyWeakModifier;
}
// If we don't have at least one mixedMeasure, the LongNameHandler would be
// sufficient and we shouldn't be running MixedUnitLongNameHandler code:
U_ASSERT(micros.mixedMeasuresCount > 0);
// mixedMeasures does not contain the last value:
U_ASSERT(fMixedUnitCount == micros.mixedMeasuresCount + 1);
@ -486,6 +488,11 @@ const Modifier *MixedUnitLongNameHandler::getMixedUnitModifier(DecimalQuantity &
for (int32_t i = 0; i < micros.mixedMeasuresCount; i++) {
DecimalQuantity fdec;
fdec.setToLong(micros.mixedMeasures[i]);
if (i > 0 && fdec.isNegative()) {
// If numbers are negative, only the first number needs to have its
// negative sign formatted.
fdec.negate();
}
StandardPlural::Form pluralForm = utils::getStandardPlural(rules, fdec);
UnicodeString simpleFormat =
@ -499,6 +506,13 @@ const Modifier *MixedUnitLongNameHandler::getMixedUnitModifier(DecimalQuantity &
// TODO(icu-units#67): fix field positions
}
// Reiterated: we have at least one mixedMeasure:
U_ASSERT(micros.mixedMeasuresCount > 0);
// Thus if negative, a negative has already been formatted:
if (quantity.isNegative()) {
quantity.negate();
}
UnicodeString *finalSimpleFormats = &fMixedUnitData[(fMixedUnitCount - 1) * ARRAY_LENGTH];
StandardPlural::Form finalPlural = utils::getPluralSafe(micros.rounder, rules, quantity, status);
UnicodeString finalSimpleFormat = getWithPlural(finalSimpleFormats, finalPlural, status);

10
icu4c/source/i18n/units_complexconverter.cpp
View file

@ -110,9 +110,13 @@ UBool ComplexUnitsConverter::greaterThanOrEqual(double quantity, double limit) c
MaybeStackVector<Measure> ComplexUnitsConverter::convert(double quantity,
icu::number::impl::RoundingImpl *rounder,
UErrorCode &status) const {
// TODO(icu-units#63): test negative numbers!
// TODO(hugovdm): return an error for "foot-and-foot"?
MaybeStackVector<Measure> result;
int sign = 1;
if (quantity < 0) {
quantity *= -1;
sign = -1;
}
// For N converters:
// - the first converter converts from the input unit to the largest unit,
@ -190,7 +194,7 @@ MaybeStackVector<Measure> ComplexUnitsConverter::convert(double quantity,
// Package values into Measure instances in result:
for (int i = 0, n = unitConverters_.length(); i < n; ++i) {
if (i < n - 1) {
Formattable formattableQuantity(intValues[i]);
Formattable formattableQuantity(intValues[i] * sign);
// Measure takes ownership of the MeasureUnit*
MeasureUnit *type = new MeasureUnit(units_[i]->copy(status).build(status));
if (result.emplaceBackAndCheckErrorCode(status, formattableQuantity, type, status) ==
@ -204,7 +208,7 @@ MaybeStackVector<Measure> ComplexUnitsConverter::convert(double quantity,
}
} else { // LAST ELEMENT
// Add the last element, not an integer:
Formattable formattableQuantity(quantity);
Formattable formattableQuantity(quantity * sign);
// Measure takes ownership of the MeasureUnit*
MeasureUnit *type = new MeasureUnit(units_[i]->copy(status).build(status));
if (result.emplaceBackAndCheckErrorCode(status, formattableQuantity, type, status) ==

3
icu4c/source/i18n/units_router.cpp
View file

@ -15,6 +15,7 @@
#include "unicode/measure.h"
#include "units_data.h"
#include "units_router.h"
#include <cmath>
U_NAMESPACE_BEGIN
namespace units {
@ -96,7 +97,7 @@ RouteResult UnitsRouter::route(double quantity, icu::number::impl::RoundingImpl
const ConverterPreference *converterPreference = nullptr;
for (int32_t i = 0, n = converterPreferences_.length(); i < n; i++) {
converterPreference = converterPreferences_[i];
if (converterPreference->converter.greaterThanOrEqual(quantity * (1 + DBL_EPSILON),
if (converterPreference->converter.greaterThanOrEqual(std::abs(quantity) * (1 + DBL_EPSILON),
converterPreference->limit)) {
break;
}

37
icu4c/source/test/intltest/numbertest_api.cpp
View file

@ -764,6 +764,24 @@ void NumberFormatterApiTest::unitMeasure() {
Locale("en-US"),
1.9999,
u"2 feet, 0 inches");
assertFormatSingle(
u"Negative numbers: temperature",
u"measure-unit/temperature-celsius",
u"unit/celsius",
NumberFormatter::with().unit(MeasureUnit::forIdentifier("celsius", status)),
Locale("nl-NL"),
-6.5,
u"-6,5\u00B0C");
assertFormatSingle(
u"Negative numbers: time",
nullptr, // submitting after TODO(icu-units#35) is fixed: fill in skeleton!
u"unit/hour-and-minute-and-second",
NumberFormatter::with().unit(MeasureUnit::forIdentifier("hour-and-minute-and-second", status)),
Locale("de-DE"),
-1.24,
u"-1 Std., 14 Min. und 24 Sek.");
}
void NumberFormatterApiTest::unitCompoundMeasure() {
@ -863,6 +881,16 @@ void NumberFormatterApiTest::unitCompoundMeasure() {
2.4,
"2.4 m/s/s");
assertFormatSingle(
u"Negative numbers: acceleration",
u"measure-unit/acceleration-meter-per-square-second",
// TODO: when other PRs are merged, try: u"unit/meter-per-second-second" instead:
u"measure-unit/acceleration-meter-per-square-second",
NumberFormatter::with().unit(MeasureUnit::forIdentifier("meter-per-pow2-second", status)),
Locale("af-ZA"),
-9.81,
u"-9,81 m/s\u00B2");
// Testing the rejection of invalid specifications
// If .unit() is not given a built-in type, .perUnit() is not allowed
@ -1134,6 +1162,15 @@ void NumberFormatterApiTest::unitUsage() {
u"8,765E0 square metres",
u"0E0 square centimetres");
assertFormatSingle(
u"Negative numbers: minute-and-second",
u"measure-unit/duration-second usage/media",
u"unit/second usage/media",
NumberFormatter::with().unit(SECOND).usage("media"),
Locale("nl-NL"),
-77.7,
u"-1 min, 18 sec");
assertFormatSingle(
u"Rounding Mode propagates: rounding down",
u"usage/road measure-unit/length-centimeter rounding-mode-floor",

14
icu4j/main/classes/core/src/com/ibm/icu/impl/number/MixedUnitLongNameHandler.java
View file

@ -141,6 +141,8 @@ public class MixedUnitLongNameHandler
* unit.
*/
private Modifier getMixedUnitModifier(DecimalQuantity quantity, MicroProps micros) {
// If we don't have at least one mixedMeasure, the LongNameHandler would be
// sufficient and we shouldn't be running MixedUnitLongNameHandler code:
if (micros.mixedMeasures.size() == 0) {
assert false : "Mixed unit: we must have more than one unit value";
throw new UnsupportedOperationException();
@ -168,6 +170,11 @@ public class MixedUnitLongNameHandler
for (int i = 0; i < micros.mixedMeasures.size(); i++) {
DecimalQuantity fdec = new DecimalQuantity_DualStorageBCD(micros.mixedMeasures.get(i).getNumber());
if (i > 0 && fdec.isNegative()) {
// If numbers are negative, only the first number needs to have its
// negative sign formatted.
fdec.negate();
}
StandardPlural pluralForm = fdec.getStandardPlural(rules);
String simpleFormat = LongNameHandler.getWithPlural(this.fMixedUnitData.get(i), pluralForm);
@ -179,6 +186,13 @@ public class MixedUnitLongNameHandler
// TODO(icu-units#67): fix field positions
}
// Reiterated: we have at least one mixedMeasure:
assert micros.mixedMeasures.size() > 0;
// Thus if negative, a negative has already been formatted:
if (quantity.isNegative()) {
quantity.negate();
}
String[] finalSimpleFormats = this.fMixedUnitData.get(this.fMixedUnitData.size() - 1);
StandardPlural finalPlural = RoundingUtils.getPluralSafe(micros.rounder, rules, quantity);
String finalSimpleFormat = LongNameHandler.getWithPlural(finalSimpleFormats, finalPlural);

9
icu4j/main/classes/core/src/com/ibm/icu/impl/units/ComplexUnitsConverter.java
View file

@ -105,6 +105,11 @@ public class ComplexUnitsConverter {
*/
public List<Measure> convert(BigDecimal quantity, Precision rounder) {
List<Measure> result = new ArrayList<>(unitConverters_.size());
BigDecimal sign = BigDecimal.ONE;
if (quantity.compareTo(BigDecimal.ZERO) < 0) {
quantity = quantity.abs();
sign = sign.negate();
}
// For N converters:
// - the first converter converts from the input unit to the largest
@ -179,9 +184,9 @@ public class ComplexUnitsConverter {
// Package values into Measure instances in result:
for (int i = 0, n = unitConverters_.size(); i < n; ++i) {
if (i < n - 1) {
result.add(new Measure(intValues.get(i), units_.get(i).build()));
result.add(new Measure(intValues.get(i).multiply(sign), units_.get(i).build()));
} else {
result.add(new Measure(quantity, units_.get(i).build()));
result.add(new Measure(quantity.multiply(sign), units_.get(i).build()));
}
}

3
icu4j/main/classes/core/src/com/ibm/icu/impl/units/UnitsRouter.java
View file

@ -86,7 +86,8 @@ public class UnitsRouter {
ConverterPreference converterPreference = null;
for (ConverterPreference itr : converterPreferences_) {
converterPreference = itr;
if (converterPreference.converter.greaterThanOrEqual(quantity, converterPreference.limit)) {
if (converterPreference.converter.greaterThanOrEqual(quantity.abs(),
converterPreference.limit)) {
break;
}
}

37
icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java
View file

@ -726,6 +726,24 @@ public class NumberFormatterApiTest extends TestFmwk {
new ULocale("en-US"),
1.9999,
"2 feet, 0 inches");
assertFormatSingle(
"Negative numbers: temperature",
"measure-unit/temperature-celsius",
"unit/celsius",
NumberFormatter.with().unit(MeasureUnit.forIdentifier("celsius")),
new ULocale("nl-NL"),
-6.5,
"-6,5\u00B0C");
assertFormatSingle(
"Negative numbers: time",
null, // submitting after TODO(icu-units#35) is fixed: fill in skeleton!
"unit/hour-and-minute-and-second",
NumberFormatter.with().unit(MeasureUnit.forIdentifier("hour-and-minute-and-second")),
new ULocale("de-DE"),
-1.24,
"-1 Std., 14 Min. und 24 Sek.");
}
@Test
@ -824,6 +842,16 @@ public class NumberFormatterApiTest extends TestFmwk {
2.4,
"2.4 m/s/s");
assertFormatSingle(
"Negative numbers: acceleration",
"measure-unit/acceleration-meter-per-square-second",
// TODO: when other PRs are merged, try: u"unit/meter-per-second-second" instead:
"measure-unit/acceleration-meter-per-square-second",
NumberFormatter.with().unit(MeasureUnit.forIdentifier("meter-per-pow2-second")),
new ULocale("af-ZA"),
-9.81,
"-9,81 m/s\u00B2");
// Testing the rejection of invalid specifications
// If .unit() is not given a built-in type, .perUnit() is not allowed
@ -1091,6 +1119,15 @@ public class NumberFormatterApiTest extends TestFmwk {
"8,765E0 square metres",
"0E0 square centimetres");
assertFormatSingle(
"Negative numbers: minute-and-second",
"measure-unit/duration-second usage/media",
"unit/second usage/media",
NumberFormatter.with().unit(MeasureUnit.SECOND).usage("media"),
new ULocale("nl-NL"),
-77.7,
"-1 min, 18 sec");
assertFormatSingle(
"Rounding Mode propagates: rounding down",
"usage/road measure-unit/length-centimeter rounding-mode-floor",