-
Notifications
You must be signed in to change notification settings - Fork 387
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-18155 Order languageData's scripts by number of users #4237
CLDR-18155 Order languageData's scripts by number of users #4237
Conversation
757c108
to
cbce758
Compare
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
@@ -1346,7 +1346,7 @@ XXX Code for transations where no currency is involved | |||
<language type="awa" scripts="Deva"/> | |||
<language type="awa" territories="IN" alt="secondary"/> | |||
<language type="ay" scripts="Latn" territories="BO"/> | |||
<language type="az" scripts="Arab Cyrl Latn" territories="AZ"/> | |||
<language type="az" scripts="Arab Latn Cyrl" territories="AZ"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
likely subtags for az is az_Latn_AZ but I will note there are more Azerbaijani speakers in Iran (az_Arab_IR). It's just that they haven't transitioned to the internet like Azerbaijan has.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While more speakers, it appears that there is more written in Latin (AZ) that Arabic (IR). So probably better Latn Arab Cyrl.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This language points out a good flaw in population estimates alone -- and it depends on the usage.
Do people want likely subtags based on the largest corpora, for active internet users, for official government mandates, for any potential user (regardless of online status), for spoken content, for local content v global content (eg. AE's largest internet usage is en_AE not ar_AE).
An interesting idea (for LikelySubtags) could be to add alt="official" and potentially other alt tags when it's different. But yes, let's just stew on this thought and return to it if/when we have a good evidence it would be useful downstream.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strong agreement with what Mark said: despite spoken Azerbaijani being quite common in Iran with more speakers than in Azerbaijan, literacy in the writing system is pretty low in Iran, due to it not being formally taught in schools. There are books and magazines published in az-Arab in Iran, but most Iranian Azerbaijani speakers have difficulty reading them, even if they are literate in Persian (which is is theoretically a close writing system). So if anyone runs into written Azerbaijani in the wild these days, there's a much higher chance it's in Latin than in Arabic.
I also believe there's much more spoken "content" (read "media") produced in the language in Azerbaijan than in Iran (there's limited Azerbaijani radio and TV stations in Iran, and I believe less podcasts and songs produced in Iran). But either way, I don't think we want to tag such content as az-Arab or az-Latn. That's just az (or azb/azj if you insist on using ISO 639-3), and not az-Latn or az-Arab.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedbacks -- in the meetings last week Mark recommended putting the script from Likely Subtags first, regardless of population. So this will now read as Latn Arab Cyrl
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added in-line comments for some languages with interesting changes. Otherwise they all match likely subtags and public records.
tools/cldr-code/src/main/java/org/unicode/cldr/util/SupplementalDataInfo.java
Outdated
Show resolved
Hide resolved
for (String script : scripts) { | ||
scriptsByPopulation.put(script, 0); | ||
} | ||
return setScripts(scriptsByPopulation); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to build once, make immutable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know generally what you mean but I'm not sure how you want me to do this. Do you want me to avoid calling setScripts and include the code here as well? Personally I prefer DRY code over WET.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked this over more, sorry for being so terse.
All the data from SupplementalDataInfo needs to be immutable; that is, you must not be able to change an object you fetch from it and have that modify what SupplementalDataInfo does.
Now, the BasicLanguageData data is collected and accessed via languageToBasicLanguageData, which is protected via
1,206: CldrUtility.protectCollection(languageToBasicLanguageData);
So as long as BasicLanguageData is Freezable, and Freeze is correctly implemented, we are in good shape.
However, you add a new field scriptsByPopulation, which is not protected in the freeze() method. If any process can get access to scriptsByPopulation, then it needs to be.
That is, if scriptsByPopulation can be accessed by any public call to a BasicLanguageData call OR it can be modified by any public call, then it needs to be protected in the freeze call.
It appears that it can be, by at least one call, addScript.
cbce758
to
f03fd8c
Compare
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok to merge. Can do fixups later if necessary
f03fd8c
to
341ff39
Compare
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
341ff39
to
0a85ccb
Compare
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
In the latest review Mark mentioned that he linked the idea of regardless of population, the current likely subtag should be first in this list. This change makes that happen. This mostly affects historic languages but also a few with known overrides to their scripts. Changed languages: * Azerbaijani [az] gets Latin first. There are more Azerbaijani speakers in Iran (thereby using the Arabic script) but the online precense of Azeris is centered in Azerbaijan (thereby Latin script) * Lingua Franca Nova [lfn], a constructed language, gets Latin first * Panjabi [pa] (currently likely subtag to Gurmukhī script in India) no longer has Perso-Arabic first, because of online usage is different than populations * Pali [pi] gets Sinhala first in the secondary tags -- in fact though I need to fix this with my other script changes * Samaritan Aramaic [sam], an extinct language gets the Samaritan alphabet first * Old Irish [sga] gets Ogham writing first * Umbrian [xum] swaps to get Latin first but I'm also not sure this is correct -- but it is the current value in Likely subtags so I'll leave it be (for now).
0a85ccb
to
7e09402
Compare
Hooray! The files in the branch are the same across the force-push. 😃 ~ Your Friendly Jira-GitHub PR Checker Bot |
@@ -2198,9 +2198,9 @@ XXX Code for transations where no currency is involved | |||
<language type="sel" scripts="Cyrl"/> | |||
<language type="ses" scripts="Latn"/> | |||
<language type="sg" scripts="Latn" territories="CF"/> | |||
<language type="sga" scripts="Latn Ogam" alt="secondary"/> | |||
<language type="sga" scripts="Ogam Latn" alt="secondary"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this take into account the volume of Old Irish text that is in Latn? (It may, just asking.)
The primary goal is to reflect current usage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking a look. I believe you are correct, the likely subtag for Old Irish [sga] is wrong, it should be Latn
not Ogam
-- do you want me to fix that now or as part of the task to audit primary scripts (and thereby likely subtags) https://unicode-org.atlassian.net/browse/CLDR-18121
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are sure the fixes can be applied before release, it's fine to do them in a separate ticket.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Er, I'll fix Old Irish RIGHT now since we are close to the v47 cut.
Ah, even if Nko is a minority script it still has to be regarded as the highest with other data we have.
5189e1d
to
a1ea853
Compare
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
Note: I filed a related ticket https://unicode-org.atlassian.net/browse/CLDR-18224 |
CLDR-18155
This sorts the scripts in the SupplementalData tags such that the first script has the highest population. This helps resolve some of the ambiguities interpreting the data.
ALLOW_MANY_COMMITS=true