From cc0f8a75be5562c3632494861032d2e05df4926f Mon Sep 17 00:00:00 2001 From: Peter Edberg Date: Wed, 12 Feb 2025 09:06:21 -0800 Subject: [PATCH] CLDR-15693 Improve CurrencyDateInfo ordering, add test for XML file ordering, fix data - Order CurrencyDataInfo first by isLegalTender, then by newest date range (not older) - Add test for currencyData/region element ordering (region code, tender) - Fix supplementalData.xml currencyData to pass test - Document that currency elements with tender="false" should be last in list for currencyData/region --- common/supplemental/supplementalData.xml | 6 +- docs/ldml/tr35-numbers.md | 2 + docs/ldml/tr35.md | 2 + .../cldr/util/SupplementalDataInfo.java | 19 +++- .../cldr/unittest/TestSupplementalInfo.java | 88 +++++++++++++++++++ 5 files changed, 113 insertions(+), 4 deletions(-) diff --git a/common/supplemental/supplementalData.xml b/common/supplemental/supplementalData.xml index 8d17aa23b74..97d23eb22f2 100644 --- a/common/supplemental/supplementalData.xml +++ b/common/supplemental/supplementalData.xml @@ -1145,12 +1145,12 @@ The printed version of ISO-4217:2001 - - - + + + diff --git a/docs/ldml/tr35-numbers.md b/docs/ldml/tr35-numbers.md index 50534a1e2e0..7d665c61514 100644 --- a/docs/ldml/tr35-numbers.md +++ b/docs/ldml/tr35-numbers.md @@ -1000,6 +1000,8 @@ That is, each `currency` element will list an interval in which it was valid. Th ``` +All `currency` elements with `tender="false"` should be at the end of the list for a given `region`. + The `from` element is limited by the fact that ISO 4217 does not go very far back in time, so there may be no ISO code for the previous currency. Currencies change relatively frequently. There are different types of changes: diff --git a/docs/ldml/tr35.md b/docs/ldml/tr35.md index 9c39ce79a09..b10fe3afc39 100644 --- a/docs/ldml/tr35.md +++ b/docs/ldml/tr35.md @@ -4347,6 +4347,8 @@ Other contributors to CLDR are listed on the [CLDR Project Page](https://www.uni ### Number symbols and formats without numberSystem - Noted that [Number Symbols](tr35-numbers.md#Number_Symbols) and [Number Formats](tr35-numbers.md#Number_Formats) without a specific `numberSystem` attribute should not be used and will be deprecated in CLDR v48. +### Clarified `currencyData` element ordering +- In [Supplemental Currency Data](tr35-numbers.md#Supplemental_Currency_Data), noted that in the list of `currency` elements for a given `currencyData/region`, those with `tender="false"` should be at the end of the list. **Changes in LDML Version 46.1 (Differences from Version 46)** diff --git a/tools/cldr-code/src/main/java/org/unicode/cldr/util/SupplementalDataInfo.java b/tools/cldr-code/src/main/java/org/unicode/cldr/util/SupplementalDataInfo.java index 94295daef18..79c121b67db 100644 --- a/tools/cldr-code/src/main/java/org/unicode/cldr/util/SupplementalDataInfo.java +++ b/tools/cldr-code/src/main/java/org/unicode/cldr/util/SupplementalDataInfo.java @@ -703,8 +703,25 @@ public boolean isLegalTender() { @Override public int compareTo(CurrencyDateInfo o) { + // Sort first by isLegalTender + if (isLegalTender && !o.isLegalTender) { + return -1; + } + if (!isLegalTender && o.isLegalTender) { + return 1; + } + // Note that sorting using criteria other than the above loses info, because per + // https://www.unicode.org/reports/tr35/tr35-numbers.html#Supplemental_Currency_Data, + // "The [xml file] *ordering* of the elements in the list tells us which was the + // primary currency during any period in time." However we do keep the further + // comparison steps below to preserve a unique ordering of CurrencyDateInfo items; we + // just make the ordering closer to the file order by sorting the dateRange newest + // first (CLDR-15693). + // + // Then sort by date range int result = dateRange.compareTo(o.dateRange); - if (result != 0) return result; + if (result != 0) return -result; // want *newest* "to" first, then newest "from" + // then by currency return currency.compareTo(o.currency); } diff --git a/tools/cldr-code/src/test/java/org/unicode/cldr/unittest/TestSupplementalInfo.java b/tools/cldr-code/src/test/java/org/unicode/cldr/unittest/TestSupplementalInfo.java index 8ecfbe78479..60af792545f 100644 --- a/tools/cldr-code/src/test/java/org/unicode/cldr/unittest/TestSupplementalInfo.java +++ b/tools/cldr-code/src/test/java/org/unicode/cldr/unittest/TestSupplementalInfo.java @@ -56,6 +56,7 @@ import org.unicode.cldr.util.CLDRFile; import org.unicode.cldr.util.CLDRFile.WinningChoice; import org.unicode.cldr.util.CLDRLocale; +import org.unicode.cldr.util.CLDRPaths; import org.unicode.cldr.util.CLDRURLS; import org.unicode.cldr.util.CldrUtility; import org.unicode.cldr.util.DateConstants; @@ -98,6 +99,8 @@ import org.unicode.cldr.util.SupplementalDataInfo.SampleList; import org.unicode.cldr.util.Validity; import org.unicode.cldr.util.Validity.Status; +import org.unicode.cldr.util.XMLFileReader; +import org.unicode.cldr.util.XPathParts; public class TestSupplementalInfo extends TestFmwkPlus { static CLDRConfig testInfo = CLDRConfig.getInstance(); @@ -1752,6 +1755,91 @@ public void TestCurrencyDecimalPlaces() { } } + private List> currencyDataRegionEntries = + new ArrayList>(); + + public void TestCurrencyDataOrder() { + // We need to check the order of currencyData/region entries in the xml file, so use + // XMLFileReader to get a list of the necessary item data in file order instead of + // accessing through SupplementalDataInfo + CurrencyDataHandler currencyDataHandler = new CurrencyDataHandler(); + XMLFileReader xfr = new XMLFileReader().setHandler(currencyDataHandler); + String pathToSupplemental = + CLDRPaths.DEFAULT_SUPPLEMENTAL_DIRECTORY + "supplementalData.xml"; + xfr.read(pathToSupplemental, -1, true); + currencyDataHandler.cleanup(); + + // Now check the list order + R3 lastEntry = Row.of("", null, ""); + for (R3 entry : currencyDataRegionEntries) { + String iso3166 = entry.get0(); + String lastIso3166 = lastEntry.get0(); + int compareRegions = iso3166.compareTo(lastIso3166); + if (compareRegions < 0) { + String path = entry.get2(); + errln( + " currencyData/region " + + iso3166 + + " not after " + + lastIso3166 + + "; path: " + + path); + } else if (compareRegions == 0) { + // Check entries within a given region + CurrencyDateInfo currencyDateInfo = entry.get1(); + CurrencyDateInfo lastCurrencyDateInfo = lastEntry.get1(); + if (currencyDateInfo.isLegalTender() && !lastCurrencyDateInfo.isLegalTender()) { + String path = entry.get2(); + errln( + " For currencyData/region " + + iso3166 + + ", entries with tender=false should be last; path: " + + path); + } + // We do not enforce any other ordering of currency entries (i.e. by date range + // and/or currency code) since that ordering carries meaning; per + // https://www.unicode.org/reports/tr35/tr35-numbers.html#Supplemental_Currency_Data, + // "The [xml file] *ordering* of the elements in the list tells us which was the + // primary currency during any period in time." + } // else we are starting a new region in order + lastEntry = entry; + } + } + + class CurrencyDataHandler extends XMLFileReader.SimpleHandler { + public void cleanup() {} // Finish processing anything left in the file + + @Override + public void handlePathValue(String path, String value) { + try { + XPathParts parts = XPathParts.getFrozenInstance(path); + if (parts.size() < 4) { + return; + } + String currencyData = parts.getElement(1); + String region = parts.getElement(2); + String currency = parts.getElement(3); + if (!currencyData.equals("currencyData") + || !region.equals("region") + || !currency.equals("currency")) { + return; + } + String iso3166 = parts.getAttributeValue(2, "iso3166"); // required + String iso4217 = parts.getAttributeValue(3, "iso4217"); // required + String fromDate = parts.getAttributeValue(3, "from"); // optional + String toDate = parts.getAttributeValue(3, "to"); // optional + String tender = parts.getAttributeValue(3, "tender"); // optional + CurrencyDateInfo currencyDateInfo = + new CurrencyDateInfo(iso4217, fromDate, toDate, tender); + currencyDataRegionEntries.add(Row.of(iso3166, currencyDateInfo, path)); + } catch (Exception e) { + throw (IllegalArgumentException) + new IllegalArgumentException("path: " + path + ",\tvalue: " + value) + .initCause(e); + } + } + } + /** Verify that we have a default script for every CLDR base language */ public void TestDefaultScripts() { SupplementalDataInfo supp = SUPPLEMENTAL;