Skip to content
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

CLDR-15693 Improve CurrencyDateInfo ordering, add test for XML file ordering, fix data #4374

Conversation

pedberg-icu
Copy link
Contributor

@pedberg-icu pedberg-icu commented Feb 13, 2025

CLDR-15693

  • This PR completes the ticket.

Note that per TR35 Supplemental Currency Data, "The [xml file] ordering of the elements in the [currency] list tells us which was the primary currency during any period in time."

  • Update TR35 to note that currency elements with tender="false" should be last in list for currencyData/region (they are never primary at any point in time).
  • Order SupplementalDataInfo.CurrencyDataInfo first by isLegalTender, then by newest date range first (not older date range). Note that while this ordering is closer to the file ordering, it will not be identical (since we need to sort these in a unique way), so SupplementalDataInfo has never preserved info about the primary currency during any time period in the event that two currencies are legal tender during overlapping time periods.
  • Add test for currencyData/region element ordering (regions must be ordered by region code, and within a region, currency elements with tender = "false" should be last).
  • Fix supplementalData.xml currencyData to pass test (just reorder one region)

ALLOW_MANY_COMMITS=true

…rdering, 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
@pedberg-icu pedberg-icu self-assigned this Feb 13, 2025
@pedberg-icu
Copy link
Contributor Author

@macchiati @srl295 I seem to have lost the ability to merge PRs ("You’re not authorized to merge this pull request"). Can one of you merge this for now, then we can figure out what is going on? Thanks.

@srl295 srl295 merged commit 0899783 into unicode-org:main Feb 13, 2025
13 checks passed
@srl295
Copy link
Member

srl295 commented Feb 13, 2025

@pedberg-icu I sure thought I had re-added you but I will double check a little bit later tonight

@pedberg-icu pedberg-icu deleted the CLDR-15693-CurrencyDateInfo-to-sort-tender-false-last branch February 14, 2025 00:04
@pedberg-icu
Copy link
Contributor Author

pedberg-icu commented Feb 14, 2025

@pedberg-icu I sure thought I had re-added you but I will double check a little bit later tonight

@srl295 Should I have rebased onto current main? Did the deleted cldr-tc group affect the actual cldr repo such that I might have branched from the bad version?.... Does not look like it; and I do seem to be in the current version of https://github.com/orgs/unicode-org/teams/cldr-tc

@srl295
Copy link
Member

srl295 commented Feb 14, 2025

@pedberg-icu I sure thought I had re-added you but I will double check a little bit later tonight

@srl295 Should I have rebased onto current main?

no impact on this

Did the deleted cldr-tc group affect the actual cldr repo such that I might have branched from the bad version?

no, just affected your perms

.... Does not look like it; and I do seem to be in the current version of https://github.com/orgs/unicode-org/teams/cldr-tc

yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants