cldr
cldr copied to clipboard
CLDR-18465 Remove Before Diocletian era from Coptic calendar
CLDR-18465
- [ ] This PR completes the ticket.
Related to #4581. Wanted to get feedback on how to remove an era.
@pedberg-icu @srl295
ALLOW_MANY_COMMITS=true
Please note that I did not go through and delete all of the existing BD data. Should I?
What are the next steps on this PR?
Hi, can I have a review on this PR?
For normal tickets (eg non-infrastructure) the process is to get them accepted in the next TC meeting, to make sure that there is visibility into substantive changes. So can you add this to next week's CLDR agenda?
(and btw, the tests are failing.)
The Design WG said on 2025-05-05:
Recommendation: 2.b.ii +
Keep type values and codes stable in CLDR; same meaning across time.
No gaps, no overlaps in datetime span.
Abandon zero-based and ascending order: don’t care about gaps or order; numbers act like string keys.
In formatting, years before first (in time) are negative, years after end just extend.
2.b.ii says
Add an era type key in locale display name data – a string key, forming a map=dict, not just an array
I don't actually know what that part means.
So what is the intended implementation of this PR? Is it to delete all data for era 0 and leave the data for era 1 as-is, or is it something more than that?
Hi, can I get a signal on how I should proceed with this PR?
@macchiati @pedberg-icu @roozbehp @FrankYFTang
Please, I want to land this and its dependent PRs in CLDR 48 but I've been blocked for months on reviewer feedback.
is this still targeted for v48?
Please, I want to land this and its dependent PRs in CLDR 48 but I've been blocked for months on reviewer feedback.
It's also still failing tests
I'm holding off on merging this until the ICU change sticks (ICU-23172 which apparently has memory leaks), but I would still like to see it in CLDR 48.
The test failures say
2025-08-27T00:48:28.7632457Z TestAllLocales
2025-08-27T00:48:28.7636883Z ##[error] (TestCheckCLDR.java:503) Error: : bs //ldml/dates/calendars/calendar[@type="coptic"]/eras/eraAbbr/era[@type="0"]: expected != null
@srl295, can you suggest how to go about fixing that?
I'm holding off on merging this until the ICU change sticks (ICU-23172 which apparently has memory leaks), but I would still like to see it in CLDR 48.
@sffc Those memory leaks were fixed, along with an inheritance issue for narrow eras that should have inherited from root, and the PR was merged. Many of the memory leaks were actually in logKnownIssue test skips, nothing to do with the eraNames stuff.
Test is here: https://github.com/unicode-org/cldr/blob/58e2f6f7889304691066458bd370c74d6df94463/tools/cldr-code/src/test/java/org/unicode/cldr/unittest/TestCheckCLDR.java#L503
Looks like there are inherited values in bs.xml for example: https://github.com/unicode-org/cldr/blob/58e2f6f7889304691066458bd370c74d6df94463/common/main/bs.xml#L2963
@sffc - maybe you need to remove all the values for that era across all of the locales files? There is a CLDR tool if it's helpful: https://cldr.unicode.org/development/cldr-big-red-switch/cldrmodify-using-config-file
I'm trying but running into multiple roadblocks in trying to run the CLDRModify tool. I booked some time tomorrow to work through it with you.
Thanks @macchiati and @AEApple for your help with the CLDRModify tool. I've updated the PR.
@sffc - Can you include the CLDR modify config file in the PR? We like including them so we have a history of the CLDR modify command that was run to generate the data changes. Thanks!
Still some test failures:
TestCheckCLDR {
Test14866
(0.137s) Passed
TestA (0.720s) Passed
TestALLOWED_IN_LIMITED_PATHS (0.000s) Passed
TestAllLocales
Error: (TestCheckCLDR.java:503) Error: : bs //ldml/dates/calendars/calendar[@type="coptic"]/eras/eraAbbr/era[@type="0"]: expected != null
Error: (TestCheckCLDR.java:503) Error: : bs //ldml/dates/calendars/calendar[@type="coptic"]/eras/eraNarrow/era[@type="0"]: expected != null
Error: (TestCheckCLDR.java:503) Error: : cs //ldml/dates/calendars/calendar[@type="coptic"]/eras/eraAbbr/era[@type="0"]: expected != null
Error: (TestCheckCLDR.java:503) Error: : cs //ldml/dates/calendars/calendar[@type="coptic"]/eras/eraNarrow/era[@type="0"]: expected != null
...
It looks like it's still failing since you didn't do the following two paths for all locales:
//ldml/dates/calendars/calendar[@type="coptic"]/eras/eraAbbr/era[@type="0"] //ldml/dates/calendars/calendar[@type="coptic"]/eras/eraNarrow/era[@type="0"]
You'll need to make sure all are removed.
Yay, tests are passing!
So the CLDRModify change LGTM, but I don't know if there is any impact with re-ordering the calendars by calendar type name in English. I assume it's probably okay, but someone who knows how the calendars work should be the one to approve just in case there's some consideration that I don't know about.
It re-ordered XML that I had previously added in this same PR. The PR only adds and deletes. It does not move.
No problem with reordering.
On Mon, Sep 8, 2025, 20:33 Annemarie Apple @.***> wrote:
@.**** approved this pull request.
LGTM! You will need to squash to fix the commit before you can merge.
— Reply to this email directly, view it on GitHub https://github.com/unicode-org/cldr/pull/4620#pullrequestreview-3198953717, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACJLEMCBROKC63ZAUC73LU33RZDAVAVCNFSM6AAAAAB3JT332CVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTCOJYHE2TGNZRG4 . You are receiving this because you were mentioned.Message ID: @.***>
@sffc Is this done? It looks like the only problem is the jira ticket, so you need to squash. Can you take care of that so we can mark the ticket as done (we're trying to clean up all the alpha integration tickets.
Squashed