CLDR-17851 SupplementalCalendarData parser, update ExampleGenerator #4642

srl295 · 2025-04-25T21:03:06Z

minimal, modern parser
minimal test case
update example generator

CLDR-17851

Fixes original root cause of CLDR-18464 Fix IndexOutOfBoundsException in ExampleGenerator.handleEras #4629
This PR completes the ticket.

ALLOW_MANY_COMMITS=true

- minimal, modern parser - minimal test case - commented-out 'dump' function

- update example generator to use

- move TestEraMap into TestSDI

srl295 · 2025-04-25T21:15:47Z

@sffc this gives us a data driven approach that should be less crashy on era data changes.
I haven't put in unit tests for the data but if you have suggestions please add them to this ticket.

sffc

I don't understand enough about how CLDR works to be able to give this a thorough review

srl295 · 2025-04-25T21:21:29Z

I don't understand enough about how CLDR works to be able to give this a thorough review

mostly just a FYI that CLDR could now have data quality checks on <calendarData> (nothing, sadly, parsed it before)

srl295 · 2025-04-25T21:29:58Z

example generator issue, will bring back soon

- use new supplemental data info

- BH, added in CLDR-18464, is out of order… attempt to find the prev and next era by date.

srl295 · 2025-04-25T23:00:39Z

alright, BH now shows AH 0 as its example. Which is better than AH 1 i guess.

anyway, everyone gets examples now

srl295 · 2025-04-25T23:01:05Z

i think this is ready for review - maybe the BH is a formatter issue

AEApple · 2025-04-25T23:24:21Z

Thank you for doing this! I'm not familiar enough either to review this unfortunately, although I could try applying it in my branch and check if it fixed the error if that works?

srl295 · 2025-04-26T01:40:12Z

Already did and it does work El El vie, 25 abr 2025 a la(s) 6:24 p.m., Annemarie Apple < ***@***.***> escribió:

…

*AEApple* left a comment (unicode-org/cldr#4642) <#4642 (comment)> Thank you for doing this! I'm not familiar enough either to review this unfortunately, although I could try applying it in my branch and check if it fixed the error if that works? — Reply to this email directly, view it on GitHub <#4642 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAGQZMZJVOIWBLKJ7VMBPMT23K73ZAVCNFSM6AAAAAB34QIRLKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQMZRGU4TMNRQGI> . You are receiving this because you were assigned.Message ID: ***@***.***>

tools/cldr-code/src/main/java/org/unicode/cldr/test/ExampleGenerator.java

macchiati

A few items

tools/cldr-code/src/main/java/org/unicode/cldr/util/SupplementalCalendarData.java

macchiati · 2025-04-28T23:06:41Z

tools/cldr-code/src/main/java/org/unicode/cldr/util/SupplementalCalendarData.java

+            type = Integer.parseInt(xpath.getAttributeValue(INDEX, LDMLConstants.TYPE));
+
+            start = xpath.getAttributeValue(INDEX, LDMLConstants.START);
+            startCalendar = forDateString(start);


A Calendar is pretty heavy-weight for this. Would be simplest to just extract the millis, or use an Instant if we want type-safety

Actually, an even better reason is that then you (and your clients) can avoid complications with null. The earliest start date is Integer.MIN_VALUE and the latest end date is Integer.MAX_VALUE.

There is one further complication. Normally missing start/end dates get the above values. However, for the Japanese calendar, the end date is set based on the next line's start data - 1.
So

<era type="0" start="645-6-19"/> <era type="1" start="650-2-15"/> ... <era type="235" start="1989-1-8" code="heisei"/> <era type="236" start="2019-5-1" code="reiwa"/>

Is equivalent to

<era type="0" start="645-6-19" end="650-2-14" /> <era type="1" start="650-2-15" end="671-12-31"/> ... <era type="235" start="1989-1-8" end="2019-4-30" code="heisei"/> <era type="236" start="2019-5-1" code="reiwa"/>

So that means you don't know the end data for an era until the next one gets added. And that's assuming that you read them in file order. Otherwise you need to read them in, and sort by start date.

do we need more clarity in the spec here?

before this, i couldn't find any code in CLDR that read these lines.

So that means you don't know the end data for an era until the next one gets added. And that's assuming that you read them in file order. Otherwise you need to read them in, and sort by start date.

That's why i had the end time be null there.

macchiati · 2025-04-28T23:19:41Z

tools/cldr-code/src/main/java/org/unicode/cldr/util/SupplementalCalendarData.java

+import org.unicode.cldr.icu.LDMLConstants;
+
+public class SupplementalCalendarData implements Iterable<String> {
+    /** an <era> element */


Suggested change

/** an <era> element */

/** an <era> element, which represents the era information from a calendar such as:

* <pre>

* <calendarData><supplementalData>

* <calendar type="islamic">

* <calendarSystem type="lunar" />

* <eras>

* <era type="0" start="622-7-15" code="islamic" aliases="ah"/>

* <era type="1" end="622-7-14" code="islamic-inverse" aliases="bh"/>

* </eras>

* </calendar>

* </pre>

*/

or just link to the spec?

The behavior of start/end is documented in https://www.unicode.org/reports/tr35/tr35-69/tr35-dates.html#Calendar_Data

(It could use some wordsmithing to clarify that "open-ended" applies when there isn't a series; probably start that first. I'll file a ticket for that.)

Ticket: https://unicode-org.atlassian.net/browse/CLDR-18576

macchiati · 2025-04-28T23:32:09Z

tools/cldr-code/src/main/java/org/unicode/cldr/util/SupplementalCalendarData.java

+            type = Integer.parseInt(xpath.getAttributeValue(INDEX, LDMLConstants.TYPE));
+
+            start = xpath.getAttributeValue(INDEX, LDMLConstants.START);
+            startCalendar = forDateString(start);


Actually, an even better reason is that then you (and your clients) can avoid complications with null. The earliest start date is Integer.MIN_VALUE and the latest end date is Integer.MAX_VALUE.

There is one further complication. Normally missing start/end dates get the above values. However, for the Japanese calendar, the end date is set based on the next line's start data - 1.
So

<era type="0" start="645-6-19"/> <era type="1" start="650-2-15"/> ... <era type="235" start="1989-1-8" code="heisei"/> <era type="236" start="2019-5-1" code="reiwa"/>

Is equivalent to

<era type="0" start="645-6-19" end="650-2-14" /> <era type="1" start="650-2-15" end="671-12-31"/> ... <era type="235" start="1989-1-8" end="2019-4-30" code="heisei"/> <era type="236" start="2019-5-1" code="reiwa"/>

So that means you don't know the end data for an era until the next one gets added. And that's assuming that you read them in file order. Otherwise you need to read them in, and sort by start date.

macchiati · 2025-04-28T23:49:18Z

tools/cldr-code/src/main/java/org/unicode/cldr/util/SupplementalCalendarData.java

+        }
+
+        /** only works within the same cal system */
+        @Override


Suggested change

@Override

@Override

/* Normally comparisons require all of the fields to be listed, to ensure that no

* two unequal values a and b will never have a.compareTo(b) = 0.

* However, the structure of the survey tool guaranties that no two eras (for different calendarTypes)

* will overlap in start/end dates.

*/

the survey tool doesn't guarantee this - do you mean the spec? the unit tests ought to validate this. But I'm not sure what all of the constraints are.

Right, we don't make that clear. Filed https://unicode-org.atlassian.net/browse/CLDR-18576

You're right. It is the spec not the Survey Tool.

macchiati · 2025-04-28T23:51:16Z

tools/cldr-code/src/main/java/org/unicode/cldr/util/SupplementalCalendarData.java

+                    .compare(getLatest(), o.getLatest())
+                    .compare(getType(), o.getType())


Suggested change

.compare(getLatest(), o.getLatest())

.compare(getType(), o.getType())

.compare(getEnd(), o.getEnd())

Because of the above feature, you will never need the type for comparison; just the calendarType and end.

srl295 · 2025-04-29T01:06:20Z

@macchiati this is getting pretty heavy weight. -and not working. Could I go back to the lightweight parser that succeeds for purposes of examples and split out the heavier api questions?

srl295 · 2025-04-29T17:46:11Z

@macchiati this is getting pretty heavy weight. -and not working. Could I go back to the lightweight parser that succeeds for purposes of examples and split out the heavier api questions?

Specifically, back to Friday's version (but memoize it so the object is only created once). And split out a PR for the more heavyweight processor that tries to sort out the situation for Japanese, etc.

I was trying to make a parser that just reflected what was in the XML, to enable unit testing, but it is kind of turning into a general purpose implementation of the algorithm.

- address some concerns - split other issues out to a separate PR- unicode-org#4648

jira-pull-request-webhook · 2025-04-29T21:07:49Z

Notice: the branch changed across the force-push!

tools/cldr-code/src/main/java/org/unicode/cldr/test/ExampleGenerator.java is different
tools/cldr-code/src/main/java/org/unicode/cldr/util/DateConstants.java is no longer changed in the branch
tools/cldr-code/src/main/java/org/unicode/cldr/util/SupplementalCalendarData.java is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

- spotless

srl295 · 2025-04-29T21:15:47Z

@macchiati PTAL. This is basically what you saw Friday, plus fixed the bad import statement. As discussed, I moved the original branch over to #4648 to address the concerns.

srl295 added 2 commits April 25, 2025 16:00

CLDR-17851 SupplementalCalendarData

565b33d

- minimal, modern parser - minimal test case - commented-out 'dump' function

CLDR-17851 SupplementalCalendarData

da0ac32

- update example generator to use

srl295 self-assigned this Apr 25, 2025

CLDR-17851 Supplemental Data test fixup

3038d14

- move TestEraMap into TestSDI

srl295 mentioned this pull request Apr 25, 2025

CLDR-18253 Add en_JP #4634

Merged

1 task

srl295 requested review from macchiati, pedberg-icu, sffc and AEApple April 25, 2025 21:14

sffc reviewed Apr 25, 2025

View reviewed changes

srl295 marked this pull request as draft April 25, 2025 21:29

srl295 added 2 commits April 25, 2025 17:32

CLDR-17851 update ExampleGenerator and tests

398b9cb

- use new supplemental data info

CLDR-17851 fix for BH era

a20ea88

- BH, added in CLDR-18464, is out of order… attempt to find the prev and next era by date.

srl295 marked this pull request as ready for review April 25, 2025 23:00

AEApple requested a review from conradarcturus April 26, 2025 20:38

macchiati reviewed Apr 28, 2025

View reviewed changes

tools/cldr-code/src/main/java/org/unicode/cldr/test/ExampleGenerator.java Show resolved Hide resolved

macchiati requested changes Apr 28, 2025

View reviewed changes

macchiati reviewed Apr 28, 2025

View reviewed changes

tools/cldr-code/src/main/java/org/unicode/cldr/util/SupplementalCalendarData.java Show resolved Hide resolved

macchiati reviewed Apr 28, 2025

View reviewed changes

macchiati requested changes Apr 28, 2025

View reviewed changes

CLDR-17851 SupplementalCalendarData

0efdaee

- address some concerns - split other issues out to a separate PR- unicode-org#4648

srl295 force-pushed the cldr-17851/sdi-era-metadata branch from 3339539 to 0efdaee Compare April 29, 2025 21:07

srl295 mentioned this pull request Apr 29, 2025

🚧 Cldr 17851/sdi era metadata bis #4648

Draft

1 task

CLDR-17851 SupplementalCalendarData

9471225

- spotless

srl295 requested a review from macchiati April 29, 2025 21:11

macchiati approved these changes Apr 29, 2025

View reviewed changes

srl295 merged commit 12cd0ca into unicode-org:main Apr 29, 2025
12 checks passed

srl295 deleted the cldr-17851/sdi-era-metadata branch April 29, 2025 22:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLDR-17851 SupplementalCalendarData parser, update ExampleGenerator #4642

CLDR-17851 SupplementalCalendarData parser, update ExampleGenerator #4642

srl295 commented Apr 25, 2025 •

edited

Loading

srl295 commented Apr 25, 2025 •

edited

Loading

sffc left a comment

srl295 commented Apr 25, 2025

srl295 commented Apr 25, 2025

srl295 commented Apr 25, 2025

srl295 commented Apr 25, 2025

AEApple commented Apr 25, 2025

srl295 commented Apr 26, 2025 via email

macchiati left a comment

macchiati Apr 28, 2025 •

edited

Loading

macchiati Apr 28, 2025

srl295 Apr 29, 2025

srl295 Apr 29, 2025

macchiati Apr 28, 2025

srl295 Apr 29, 2025

macchiati Apr 29, 2025

macchiati Apr 29, 2025

macchiati Apr 28, 2025

macchiati Apr 28, 2025

srl295 Apr 29, 2025

macchiati Apr 29, 2025

macchiati Apr 29, 2025

macchiati Apr 28, 2025

srl295 commented Apr 29, 2025

srl295 commented Apr 29, 2025

jira-pull-request-webhook bot commented Apr 29, 2025

srl295 commented Apr 29, 2025

-    /** an <era> element */
+    /** an <era> element, which represents the era information from a calendar such as:
+    * <pre>
+    * <calendarData><supplementalData>
+    *    <calendar type="islamic">
+    *      <calendarSystem type="lunar" />
+    *      <eras>
+    *          <era type="0" start="622-7-15" code="islamic" aliases="ah"/>
+    *           <era type="1" end="622-7-14" code="islamic-inverse" aliases="bh"/>
+    *      </eras>
+    *   </calendar>
+    * </pre>
+    */

-        @Override
+        @Override
+        /* Normally comparisons require all of the fields to be listed, to ensure that no
+         * two unequal values a and b will never have a.compareTo(b) = 0.
+         * However, the structure of the survey tool guaranties that no two eras (for different calendarTypes)
+         * will overlap in start/end dates.
+         */

		.compare(getLatest(), o.getLatest())
		.compare(getType(), o.getType())

	.compare(getLatest(), o.getLatest())
	.compare(getType(), o.getType())
	.compare(getEnd(), o.getEnd())

CLDR-17851 SupplementalCalendarData parser, update ExampleGenerator #4642

CLDR-17851 SupplementalCalendarData parser, update ExampleGenerator #4642

Conversation

srl295 commented Apr 25, 2025 • edited Loading

srl295 commented Apr 25, 2025 • edited Loading

sffc left a comment

Choose a reason for hiding this comment

srl295 commented Apr 25, 2025

srl295 commented Apr 25, 2025

srl295 commented Apr 25, 2025

srl295 commented Apr 25, 2025

AEApple commented Apr 25, 2025

srl295 commented Apr 26, 2025 via email

macchiati left a comment

Choose a reason for hiding this comment

macchiati Apr 28, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srl295 commented Apr 29, 2025

srl295 commented Apr 29, 2025

jira-pull-request-webhook bot commented Apr 29, 2025

srl295 commented Apr 29, 2025

srl295 commented Apr 25, 2025 •

edited

Loading

srl295 commented Apr 25, 2025 •

edited

Loading

macchiati Apr 28, 2025 •

edited

Loading