CLDR-17014 Handle CURRENCY/CURRENCY_SYMBOL paths like LANGUAGE/SCRIPT/…

[btangmu]

-Use ExtraPaths instead of XMLSource.ResolvingSource.constructedItems

-Paths constructed in this way no longer have fallback values

-In ICUServiceBuilder, if currency value not found in CLDRFile, use code fallback

-In TestExampleGenerator Test4528, if currency value not found in CLDRFile, use code fallback

-In GenerateLocaleIDTestData, add comment to generated file indicating it was generated

CLDR-17014

• This PR completes the ticket.

ALLOW_MANY_COMMITS=true

[btangmu]

Test failures in TestPathHeadersAndValues. BTW, tests passed for me locally because I didn't have -e=10 in my cldrtest script, so local testing wasn't exhaustive. Now I know.

The failures seem very similar to what happened for VARIANT paths in the previous PR. There, the solution was to delete INHERITANCE_MARKER from no.xml, since there was no root or code-fallback value to inherit.

    TestPathHeadersAndValues {
      Error: (TestPaths.java:199) Locale: hi_Latn,	 Value=null, 	Path: //ldml/numbers/currencies/currency[@type="VEF"]/symbol,	 IsExtraPath: false

hi.xml (not hi_Latn.xml) has

			<currency type="VEF">
				...
				<symbol draft="contributed">↑↑↑</symbol>
				<symbol alt="narrow" draft="contributed">↑↑↑</symbol>
			</currency>

https://st.unicode.org/cldr-apps/v#/hi_Latn/C_SAmerica/4ae7a40dd1e39d05 inherits from same path in root, but it's not really root, it's code fallback

https://st.unicode.org/cldr-apps/v#/hi/C_SAmerica/4ae7a40dd1e39d05 ditto

https://st.unicode.org/cldr-apps/v#/root/C_SAmerica/4ae7a40dd1e39d05

This brings us back to https://unicode-org.atlassian.net/browse/CLDR-18294 which maybe blocks this PR

[macchiati]

You might as well fix those paths with this ticket, and then the tests would pass. (Assuming that it lists all the problem cases, and you do with -e10.

It isn't blocked by https://unicode-org.atlassian.net/browse/CLDR-18294. If anything, the reverse. Because for that to land, you would have to fix those paths by hand anyway.

[btangmu]

@macchiati I've tried removing ↑↑↑ from the paths that trigger errors. The paths occur, with ↑↑↑, in locales "hi" and "no", while the errors implicate "hi_Latin" and "nn" (which sometimes inherit from "hi" and "no", respectively). The errors still occur. I'm investigating. One strange thing is here:

    public Collection<String> getExtraPaths() {
        Set<String> toAddTo = new HashSet<>(getRawExtraPaths());
        for (String path : this) {
            if ("//ldml/numbers/currencies/currency[@type=\"VEF\"]/symbol".equals(path)) {
                if (getStringValue(path) == null) {
                    System.out.println("Non-extra VEF path has null value in getExtraPaths! locale = " + getLocaleID());
                }
            }
            toAddTo.remove(path);
        }
        return toAddTo;
    }

The if and println are added temporarily for debugging. The println happens a lot, for numerous locales. That seems strange. The iteration is supposed to skip extra paths. The whole concept of "extra path" is nebulous to me. Non-extra paths typically have non-null values. My impression is, or was, that the extra-path mechanism would be the only way to get a path with null value into a CLDRFile. The result here seems to contradict that impression.

The inheritance involved in the errors seems to be from code-fallback, even though with the changes in this PR there are no longer any code-fallback values to inherit for currency paths, in the old way. This long function in XMLSource.java plays a major role:

        private AliasLocation getPathLocation(
                String xpath,
                boolean skipFirst,
                boolean skipInheritanceMarker,
                List<LocaleInheritanceInfo> list) {
...
...
...
            if (firstValue == null) {
                return new AliasLocation(xpath, CODE_FALLBACK_ID);
            } else {
                return firstValue;
            }
        }

At the end, it sort of defaults to CODE_FALLBACK_ID, for lack of any better result, seemingly regardless of whether or not any code value was found to fall back on.

[btangmu]

Another thing I noticed is that some currency paths were already implemented as "extra" paths before this PR. This code in ExtraPaths.java (formerly in CLDRFile.java) adds them on a per-locale basis, since pluralCounts is locale-specific:

    private static void addCurrencies(
            Set<String> toAddTo, Set<SupplementalDataInfo.PluralInfo.Count> pluralCounts) {
        for (String code : supplementalData.getBcp47Keys().getAll("cu")) {
            String currencyCode = code.toUpperCase();
            toAddTo.add(
                    "//ldml/numbers/currencies/currency[@type=\"" + currencyCode + "\"]/symbol");
            toAddTo.add(
                    "//ldml/numbers/currencies/currency[@type=\""
                            + currencyCode
                            + "\"]/displayName");
            if (!pluralCounts.isEmpty()) {
                for (SupplementalDataInfo.PluralInfo.Count count : pluralCounts) {
                    toAddTo.add(
                            "//ldml/numbers/currencies/currency[@type=\""
                                    + currencyCode
                                    + "\"]/displayName[@count=\""
                                    + count.toString()
                                    + "\"]");
                }
            }
        }

This overlaps with the "constant" (non-locale-specific) code that adds currency paths like this:

            addPaths(NameType.CURRENCY);
            addPaths(NameType.CURRENCY_SYMBOL);

-- based on what was formerly (before this PR) done in XMLSource.java with fallback code values.

I think this means that some paths are added twice, but that should be harmless (except maybe for performance) -- they're added to sets, not to arrays, so the duplication has no effect. I don't know how the two code sets supplementalData.getBcp47Keys().getAll("cu") and sc.getGoodAvailableCodes compare with each other (maybe identical).

[btangmu]

I've added some debugging code to this PR, including removal of some ↑↑↑ paths in locales "hi" and "no" although that didn't solve the test failures

[macchiati]

I put together a draft doc to explain the overall picture, to make sure we are all on the same page. (The goal is to create a page in https://cldr.unicode.org/development from this source (using Copy from Markdown), once the content settles down a bit.)

https://docs.google.com/document/d/1RWRGMhIYA5TOuJ1b0xvnWKUMoof0Rjyp_xGJWRs8ruw

[btangmu]

@macchiati thanks for that helpful document! It sheds a lot of light on this ticket and also https://unicode-org.atlassian.net/browse/CLDR-18217

Meanwhile I made another commit, removing some redundant addition of constant currency paths, and, having determined that the two sets supplementalData.getBcp47Keys().getAll("cu") and sc.getGoodAvailableCodes are identical aside from that latter being uppercase (size 307 BTW), calling only the latter for consistency.

[btangmu]

This was redundant, given addPaths(NameType.CURRENCY_SYMBOL)

[btangmu]

This was redundant, given addPaths(NameType.CURRENCY)

[btangmu]

getGoodAvailableCodes returns the same set of 307 codes as supplementalData.getBcp47Keys().getAll("cu") aside from being already uppercase

[btangmu]

With the 4th commit, tests pass locally! For the hi_Latn failures, it turned out that inheritance was from en_IN, so that's where some paths needed deleting. For the nn failures, it was necessary to remove a larger number of paths from no.xml -- all currency symbol paths with inheritance marker.

[btangmu]

A 5th commit was need to fix console check error "path with alt is present but the corresponding path without alt is missing". Hard-code BYN and RON as currency symbols in no.xml.

[macchiati]

Hmmm. For the currency codes, we shouldn't remove the ^^^; those are all likely to be real inheritance from the code.

That is, it is unlikely for a language to use "en" as the name of English, but not at all unlikely for it to use USD for the US currency. So rather than delete the ^^^, I think they need to be replaced by the code (for top-level locales). So what you did for BYN and RON.

[btangmu]

@macchiati Given that the currency codes are often suitable as values, is there anything wrong with the original fallback mechanism for currency paths? Maybe this PR isn't needed. What are the relative advantages of keeping code fallback, with ↑↑↑ to inherit, versus using extra paths and hard-coding the values in some top-level locales (like "no")? It might be easier or more economical just to keep code fallback.

There are also other addFallbackCode paths still remaining in constructedItems in XMLSource.java:

• 445 TZ_EXEMPLAR paths like //ldml/dates/timeZoneNames/zone[@type=...]/exemplarCity
  • code like "America/Los_Angeles"
  • value like "Los Angeles", auto-converted from code by TimezoneFormatter.getFallbackName, (red background, "untranslated code", but not an error)
• 136 paths starting with //ldml/localeDisplayNames/types/type for items in typeDisplayNames
  • path like //ldml/localeDisplayNames/types/type[@key="numbers"][@type="arab"]
  • value like "arab" ("untranslated code", red background but not an error?) or "chiffres arabes"
• 8 paths starting with //ldml/localeDisplayNames/keys/key for items in keyDisplayNames
  • path like //ldml/localeDisplayNames/keys/key[@type="calendar"]
  • value like "calendar" ("untranslated code", red background but not an error?) or "Kalender"
• 6 paths starting with //ldml/dates/calendars/calendar[@type="gregorian"]/eras/
  • path like //ldml/dates/calendars/calendar[@type="gregorian"]/eras/eraAbbr/era[@type="0"][@alt="variant"]
  • value like "BCE" (same as fallback but not described as such in ST) or "AEC"

Which (if any) of these should stay the way they are, with code fallback, and which (if any) should be changed to use the extra-path mechanism?

[macchiati]

Good question.

I think the ^^^ to CODE_FALLBACK for currency codes is still a problem, because of implementations that don't do the spec'd behavior (map to the code). One approach we could take is to add those to ROOT. But adding more conformance tests could fix that.

We should probably discuss more in Tuesday's meeting.

I think all the investigation has still been worth it (no matter what we doe), because it has exposed issues that have been hidden.

[btangmu]

I'm converting this PR to "draft", since we've (at least tentatively) concluded that the code-fallback values should be kept for currency paths. For example, the code/value "USD" for "US Dollars" is commonly used and does not trigger an error or warning. We might still want to reference this draft PR as it relates to work in progress.

I've made a separate PR #4368 copying the unproblematic changes from this PR.