[Unicode]   Common Locale Data Repository : Bug Tracking Home | Site Map | Search
 
Modify

CLDR Ticket #11611(reviewing)

Opened 7 weeks ago

Last modified 6 weeks ago

(Star) is shown in the non-winning column at the beginning of the cycle

Reported by: kristi Owned by: tbishop
Component: surveytool-UI Data Locale:
Phase: dsub Review: mark
Weeks: Data Xpath:
Xref:

Description

Star means "The item was Winning n the last release of CLDR.
However, at the very start it's showing in Others.

Attachments

Star postion.JPG (81.8 KB) - added by kristi 7 weeks ago.
T11611 2018-11-29 smoketest BCE inherited.png (205.8 KB) - added by tbishop 7 weeks ago.
three rows have inherited, first and third inherit from second
T11611 2018-11-29 smoketest fallback.png (135.3 KB) - added by tbishop 7 weeks ago.
Jump to Original leads to error with code fallback
T11611 2018-11-29 localhost BCE inherited.png (250.0 KB) - added by tbishop 7 weeks ago.
On localhost, with code changes described in comments 3/4/5, still strange

Change History

Changed 7 weeks ago by kristi

comment:1 Changed 7 weeks ago by tbishop

At the beginning of v35, it would make sense for the winning values generally to be the same as they were at the end of v34. The star should mark the winning values for v34, now known as "last-release values". Stars in the Others column seem to cry out for explanation as either exceptions or bugs.

The attached screenshot by Kristi has "BCE" in both Winning and Others columns, for https://st.unicode.org/cldr-apps/v#/ko/Gregorian/5df319d386f9eb14 . Per the Info Panel, the "BCE" in the Winning column is "the specific value currently matching the inherited value", i.e., what we call a "hard" or "explicit" value. The "BCE" in the Others column is shown as inherited from root ("soft" inheritance), and has a star for last-release value.

Ordinarily "hard" values should only appear if someone explicitly entered them, rather than voting for "soft" inheritance. In this case, however, it seems that the last-release "BCE" is being treated as "hard". Theoretically, that might be right or wrong, depending on whether "hard" really received more votes than "soft" in v34. In reality, my understanding is that ST doesn't remember the difference between hard and soft after the end of a release. Instead, I think there should only be one "BCE" item for this row, and it should in the Winning column, with star for last-release, and with colored background for inheritance, and a vote for it should be a vote for "soft" inheritance.

Another strange row is http://st.unicode.org/cldr-apps/v#/chr/Alphabetic_Information/101c4f8aa144538f (Quotation Marks, End). Where does the value U+201D RIGHT DOUBLE QUOTATION MARK in the winning column come from? Per the Info Panel the winning value is "hard". The Others column has U+0022 QUOTATION MARK, shown as inherited from root (“soft” inheritance), and with a star for last-release value. The Bailey value -- the value that would be inherited for either “hard” or “soft” inheritance -- can’t be two different values U+201D and U+0022 at the same time.

I'll study the code and data...

comment:2 Changed 7 weeks ago by mark

At the end of the release, we run CLDRModify to remove unnecessary information. That erases data in regional locales that are the same as their parent. So in the release data, there is no difference between a hard and a soft Bailey value.

Options?

  • put the star on the soft bailey value?
  • put star on both hard and soft bailey values?

comment:3 follow-up: ↓ 7 Changed 7 weeks ago by tbishop

The situation is much improved if this is added in VoteResolver.setBaileyValue:

        if (baileyValue != null && baileyValue.equals(trunkValue)) {
            trunkValue = (T) CldrUtility.INHERITANCE_MARKER;
        }

That puts the star with "BCE" ("soft", colored background) in the Winning column.

A problem remains: the Others column still has another "BCE" item ("hard"). It's there for two reasons:

(1) ballotBox.getValues(xpath) calls diskData.getValueAtDPath(xpath) and gets "BCE", which is then added as a candidate item by populateFromThisXpathAddItemsForVotes;

(2) ourSrc.getStringValue(xpath) returns "BCE"; that value is not treated as inherited since locale and sourceLocale are both "ko".

Could there be any relation to [14629] or ticket 11427?

comment:4 Changed 7 weeks ago by tbishop

The bogus "vote" in populateFromThisXpathAddItemsForVotes can be skipped as follows:

            Set<User> votes = ballotBox.getVotesForValue(xpath, avalue);
            if (votes == null || votes.size() == 0) {
                continue;
            }

That leaves "ourValue = ourSrc.getStringValue(xpath)", which maybe we should simply skip if it matches the Bailey value.

comment:5 Changed 7 weeks ago by tbishop

To skip if ourValue if matches the Bailey value, in addOurValue:

        CandidateItem myItem = null;
        if (!(ourValue != null && ourValue.equals(row.inheritedValue))) {
            myItem = row.addItem(ourValue, "ourValue");

Then if myItem is null skip subsequent parts of the function that use myItem.

With the three possible changes in comments 3/4/5 of this ticket, the first "BCE" row looks much better, but there are still other problems...

comment:6 Changed 7 weeks ago by tbishop

There are three rows (for Coverage:Comprehensive) with "BCE" at ​https://st.unicode.org/cldr-apps/v#/ko/Gregorian/5df319d386f9eb14 -- under "Eras - wide", "Eras - abbreviated", and "Eras - narrow". The first and third both inherit from the second (same page, same locale). The second inherits from root, and is displayed with red background. If you click on the second row "BCE" with red background, it shows in the Info Panel with a Jump to Original link to:

https://st.unicode.org/cldr-apps/v#/code-fallback//efe32222ec06f22

Following the link by clicking on Jump to Original leads to an error message:

"The locale, 'code-fallback', does not exist. It was either mistyped or has not been added to the Survey Tool."

I'm attaching screenshots.

Changed 7 weeks ago by tbishop

three rows have inherited, first and third inherit from second

Changed 7 weeks ago by tbishop

Jump to Original leads to error with code fallback

Changed 7 weeks ago by tbishop

On localhost, with code changes described in comments 3/4/5, still strange

comment:7 in reply to: ↑ 3 ; follow-up: ↓ 10 Changed 7 weeks ago by mark

Hmmm. Looking at

  • TODO: possibly that change should be done in the caller instead; also there may be room
  • for improvement in determining whether the last release value, when it matches the
  • inherited value, should be associated with a "hard" or "soft" candidate item.

I'm thinking the more accurate lastRelease value would be:
if the lastRelease's baileyValue(path) == lastReleaseValue(path)
then use INHERITANCE_MARKER

or maybe add an extra clause:

if the lastRelease's baileyValue(path) == lastReleaseValue(path)
&& votes(lastReleaseValue) < votes(INHERITANCE_MARKER)
then use INHERITANCE_MARKER

Replying to tbishop:

The situation is much improved if this is added in VoteResolver.setBaileyValue:

        if (baileyValue != null && baileyValue.equals(trunkValue)) {
            trunkValue = (T) CldrUtility.INHERITANCE_MARKER;
        }

That puts the star with "BCE" ("soft", colored background) in the Winning column.

A problem remains: the Others column still has another "BCE" item ("hard"). It's there for two reasons:

(1) ballotBox.getValues(xpath) calls diskData.getValueAtDPath(xpath) and gets "BCE", which is then added as a candidate item by populateFromThisXpathAddItemsForVotes;

(2) ourSrc.getStringValue(xpath) returns "BCE"; that value is not treated as inherited since locale and sourceLocale are both "ko".

Could there be any relation to [14629] or ticket 11427?

comment:8 Changed 7 weeks ago by mark

  • Owner changed from anybody to tbishop
  • Component changed from to-assess to survey-frontend

comment:9 Changed 7 weeks ago by tbishop

  • Priority changed from assess to major
  • Status changed from new to accepted
  • Milestone changed from to-assess to 35-optional

comment:10 in reply to: ↑ 7 Changed 7 weeks ago by tbishop

Replying to mark:

I'm thinking the more accurate lastRelease value would be:
...

Mark, I've copied your idea to a comment in the code for future reference. At this stage I might find it hard to determine lastRelease's baileyValue(path). See also ticket 11420, Fix inconsistent methods of getting Bailey value in Survey Tool. Maybe there should be a ticket for enabling the hard/soft inheritance distinction to carry over from one cycle to the next.

comment:11 Changed 7 weeks ago by tbishop

Another bug involves the third "BCE" row ("eraNarrow"), which inherits from second "BCE" row ("eraAbbr").

http://localhost:8080/cldr-apps/v#/ko/Gregorian/42291caf2163ca8d

We got hard BCE in Winning column, soft BCE in Others column. In STFactory.PerLocaleData.getResolverInternal when calling itself recursively, "baileyValue = r.getWinningValue()" resulted in baileyValue = INHERITANCE_MARKER when getWinningValue returned INHERITANCE_MARKER. Never set baileyValue = INHERITANCE_MARKER! If r.getWinningValue() returns INHERITANCE_MARKER, then set baileyValue = r.getBaileyValue(), which is a new function. That fixes the bug.

Last edited 7 weeks ago by tbishop (previous) (diff)

comment:12 Changed 7 weeks ago by tbishop

The Jump to Original bug ("code-fallback") has its own ticket now: 11622.

comment:13 Changed 7 weeks ago by tbishop

I've committed changes to trunk with revision [14709].

comment:14 Changed 7 weeks ago by tbishop

  • Status changed from accepted to reviewing
  • Review set to mark

comment:15 Changed 6 weeks ago by tbishop

There was one problem in revision [14709]: populateFromThisXpathAddItemsForVotes skips items in the v set without votes, but oldValue was also skipped earlier if it was in the v set.

The symptom was that adding a new value could cause the last-release value (with star) to disappear, rather than move to the Others column.

We always need to add an item for oldValue, so don't skip oldValue just because it's in v. The change is:

-        if (row.oldValue != null && !row.oldValue.equals(ourValue) && (v == null || !v.contains(row.oldValue))) {
+        if (row.oldValue != null && !row.oldValue.equals(ourValue)) {
             row.addItem(row.oldValue, "oldValue");

I committed that to trunk as revision [14716].

comment:16 Changed 6 weeks ago by tbishop

Build succeeded. Tested on smoketest. Adding new item no longer causes last-release item to disappear.

View

Add a comment

Modify Ticket

Action
as reviewing
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.