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-14428 ZoneParser fails for Australia/Currie #970

Merged
merged 1 commit into from Jan 26, 2021
Merged

CLDR-14428 ZoneParser fails for Australia/Currie #970

merged 1 commit into from Jan 26, 2021

Conversation

btangmu
Copy link
Member

@btangmu btangmu commented Jan 26, 2021

-Hard-code data for Australia/Currie in ZoneParser.java

-Also work around a possible compiler bug affecting TestSupplementalInfo.LOCALES_FIXED

Checklist

-Hard-code data for Australia/Currie in ZoneParser.java

-Also work around a possible compiler bug affecting TestSupplementalInfo.LOCALES_FIXED
@btangmu btangmu self-assigned this Jan 26, 2021
@btangmu
Copy link
Member Author

btangmu commented Jan 26, 2021

When I run CLDRModify -fp the only changes are for copyright year, no changes for Australia/Currie; that's the intention

@btangmu btangmu merged commit 9223e5b into unicode-org:master Jan 26, 2021
@srl295
Copy link
Member

srl295 commented Jan 27, 2021

@macchiati @btangmu when i merge this change into the JSON generator for #972 (using cldr-staging 38.1. data) it causes Currie to be deleted in a lot of locales, is this expected?

diff --git a/cldr-json/cldr-dates-full/main/az-Cyrl/timeZoneNames.json b/cldr-json/cldr-dates-full/main/az-Cyrl/timeZoneNames.json
index db907f21..0712b346 100644
--- a/cldr-json/cldr-dates-full/main/az-Cyrl/timeZoneNames.json
+++ b/cldr-json/cldr-dates-full/main/az-Cyrl/timeZoneNames.json
@@ -1137,9 +1137,6 @@
               "Broken_Hill": {
                 "exemplarCity": "Broken Hill"
               },
-              "Currie": {
-                "exemplarCity": "Currie"
-              },
               "Darwin": {
                 "exemplarCity": "Darwin"
               },

srl295 added a commit to unicode-org/cldr-json that referenced this pull request Jan 27, 2021
CLDR tool source: unicode-org/cldr#972

Note, 'Currie' was dropped somehow with this generation, so I excluded that:
unicode-org/cldr#970 (comment)
@btangmu
Copy link
Member Author

btangmu commented Jan 27, 2021

@macchiati @btangmu when i merge this change into the JSON generator for #972 (using cldr-staging 38.1. data) it causes Currie to be deleted in a lot of locales, is this expected?

No, I would only expect it to affect the collation of Currie, not its presence/absence

@macchiati
Copy link
Member

macchiati commented Jan 27, 2021 via email

@srl295
Copy link
Member

srl295 commented Jan 27, 2021

OK. I can dig into it some more. There's no reason to hold the CLDR JSON build for it, as it is easy enough to just revert all timeZoneNames.json files (which otherwise didn't change).

@btangmu
Copy link
Member Author

btangmu commented Jan 27, 2021

az-Cyrl.xml doesn't contain Currie or any zone elements. It presumably inherits them from az.xml, which does have zone elements including Currie. The json files, unlike the xml, explicitly include inherited values? Is there a spec/outline for how the json and xml are related and how/why they differ?

srl295 added a commit to unicode-org/cldr-json that referenced this pull request Jan 27, 2021
* cldr 38.1 beta update

v38.1.0-BETA (tagged "beta" in npm)
generated from cldr-staging 1acbe10c8af48794cbe9966cb9a6cea92cee6004
generated from cldr tools f7d6d55ca3073b209d3bf2dd9fcf15d28c259e16

* improve build script cldr-generate-json.sh

* create zipfiles

* generate cldr-segments-full and also annotations

* build scripts WIP (-V temporarily dropped)

* Update reflecting CLDR-14313 fixes

Reflecting commit c0060510ce98a43f1c8c8541afe48f3cbf442632 on CLDR
(see unicode-org/cldr#888 for its destination)

Summary of changes:
    - These items are now arrays instead of strings:
      - supplemental/calendarPreferenceData
      - supplemental/weekData (now grouped by territory)
    - Previously MISSING data now added
      - cldr-core/dayPeriods.json:    dayPeriodRuleSet-type-selection"
      - cldr-core/units.json:         unitConstants, unitQuantities
      - cldr-core/unitsMetadata.json: unit deprecations

* segments packaging missed

* script improvements: MATCH

* warn if you have a local-config.sh

* getting ready to build 38.1.0-BETA4

* update packaging 38.1.0-BETA4

* update license and versioning per CLDR-11059

CLDR tool source: unicode-org/cldr#972

Note, 'Currie' was dropped somehow with this generation, so I excluded that:
unicode-org/cldr#970 (comment)

* bump proposed tag to 38.1.0

* version regen to 38.1.0
@srl295
Copy link
Member

srl295 commented Jan 27, 2021

@btangmu that is complex… 

What i'm going to do is to split out the changes, and work on the Currie-in-JSON thing as a separate PR

@srl295
Copy link
Member

srl295 commented Jan 28, 2021

@macchiati @btangmu @yumaoka
I ran a bisect to find out where Currie was disappearing. I found that d909e3d#diff-76dbe60a9eb9c51233a3174cd4b43ef9a0fe25887970f1460e3206e3412a9423R52 (tz 2020f) shows that Australia/Currie actually is removed, since it's now a link to Hobart.

So my question is: is this a bug? Perhaps we shouldn't "Hard-code data for Australia/Currie in ZoneParser.java" if the zone is no longer in modern use on its own? Or at least, we should be using the linked data and not hard coding data?

@btangmu
Copy link
Member Author

btangmu commented Jan 28, 2021

@macchiati @btangmu @yumaoka
I ran a bisect to find out where Currie was disappearing. I found that d909e3d#diff-76dbe60a9eb9c51233a3174cd4b43ef9a0fe25887970f1460e3206e3412a9423R52 (tz 2020f) shows that Australia/Currie actually is removed, since it's now a link to Hobart.

So my question is: is this a bug? Perhaps we shouldn't "Hard-code data for Australia/Currie in ZoneParser.java" if the zone is no longer in modern use on its own? Or at least, we should be using the linked data and not hard coding data?

@srl295 We just merged this last night: #988 -- I think that answers part of your question (yes: shouldn't hard-code...)

As for "is this a bug [or a feature]", that's sort of a policy question? Mark has suggested in the follow-up ticket that CLDR translations shouldn't be allowed for deprecated zones -- so removal of Currie seems appropriate but I don't know if there are any negative consequences

@macchiati
Copy link
Member

macchiati commented Jan 28, 2021 via email

@srl295
Copy link
Member

srl295 commented Jan 28, 2021

So here's where things stand…

I wrote a test, and found that there are no time zones that don't have a country.

Do you want a test that verifies that Currie is present in the list of tz names? I'm guessing it's missing in ST also but didn't check yet.

@macchiati
Copy link
Member

macchiati commented Jan 28, 2021 via email

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