launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02765
[Merge] lp:~adeuring/launchpad/bug-688130 into lp:launchpad
Abel Deuring has proposed merging lp:~adeuring/launchpad/bug-688130 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#688130 Statistics clean-ups and extra tests
https://bugs.launchpad.net/bugs/688130
#720199 Nonsensical attempt to avoid "funnily rounded" float numbers in RosettaStats.asPercentage()
https://bugs.launchpad.net/bugs/720199
For more details, see:
https://code.launchpad.net/~adeuring/launchpad/bug-688130/+merge/51107
This branch fixes bug 688130: "Statistics clean-ups and extra tests".
Many translation statistics are calculated in POFile.updateStatistics(), which uses the two methods POFile._countTranslations() and POFile._countNewSuggestions().
These methods did not treat empty translations properly and translations which are only partially translated, i.e. those which required singlar and plural variants.
Jeroen recently fixed the case for empty translations, leaving only the partial translations: If a message is not completely translated, we want to count it as untranslated.
The SQL query in _countTranslations() now uses different expressions for the result column "has_other_msgstrs" and for the WHERE clause "has_msgstrs" to ensure that only TranslationMessage records are counted which have all required plural forms.
_countSuggestions() included translations which are diverged for another target than the POFile/POTemplate for which _countSuggestions() was called; this is fixed too.
As a drive-by fix, I added an "assert" to factory.makeCurrentTranslationMessage(): It allowed to create a translation being diverged for a POTemplate that does not contain the POTMsgSet, i.e., where no TranslationTemplateItem record exists that links the POTMsgSet with the POTemplate.
As another drive-by fix I removed a silly attempt to round a floating point number to two decimal digits from RosettaStats.asPercentage(). There are no tests for this change: The method could not round the result for example of 33/10 anyway (and of most other numbers which, represented as fraction of two integers, have a denominator with any prime factor other than two), so we either already have formatting issues or the callsites already format the values using something like '%3.2f' % (33.0/10) (bug 720199).
test: ./bin/test -vvt test_pofile
no lint
--
https://code.launchpad.net/~adeuring/launchpad/bug-688130/+merge/51107
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~adeuring/launchpad/bug-688130 into lp:launchpad.
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2011-02-19 13:26:27 +0000
+++ lib/lp/testing/factory.py 2011-02-24 11:41:50 +0000
@@ -294,6 +294,9 @@
from lp.translations.model.translationimportqueue import (
TranslationImportQueueEntry,
)
+from lp.translations.model.translationtemplateitem import (
+ TranslationTemplateItem,
+ )
from lp.translations.utilities.sanitize import (
sanitize_translations_from_webui,
)
@@ -2829,7 +2832,10 @@
if pofile is None:
pofile = self.makePOFile(language=language, side=side)
if potmsgset is None:
- potmsgset = self.makePOTMsgSet(pofile.potemplate)
+ potmsgset = self.makePOTMsgSet(pofile.potemplate, sequence=1)
+ tti_for_message_in_template = TranslationTemplateItem.selectOneBy(
+ potmsgset=potmsgset, potemplate=pofile.potemplate)
+ assert tti_for_message_in_template is not None
if translator is None:
translator = self.makePerson()
if reviewer is None:
=== modified file 'lib/lp/translations/model/pofile.py'
--- lib/lp/translations/model/pofile.py 2011-02-22 01:57:30 +0000
+++ lib/lp/translations/model/pofile.py 2011-02-24 11:41:50 +0000
@@ -844,15 +844,19 @@
side_traits = getUtility(ITranslationSideTraitsSet).getForTemplate(
self.potemplate)
+ complete_plural_clause_this_side = ' AND '.join(
+ self._appendCompletePluralFormsConditions(
+ [], table_name='Current'))
+ complete_plural_clause_other_side = ' AND '.join(
+ self._appendCompletePluralFormsConditions(
+ [], table_name='Other'))
params = {
'potemplate': quote(self.potemplate),
'language': quote(self.language),
'flag': side_traits.flag_name,
'other_flag': side_traits.other_side_traits.flag_name,
- 'has_msgstrs':
- compose_sql_translationmessage_has_translations("Current"),
- 'has_other_msgstrs':
- compose_sql_translationmessage_has_translations("Other"),
+ 'has_msgstrs': complete_plural_clause_this_side,
+ 'has_other_msgstrs': complete_plural_clause_other_side,
}
# The "distinct on" combined with the "order by potemplate nulls
# last" makes diverged messages mask their shared equivalents.
@@ -864,6 +868,7 @@
%(has_other_msgstrs)s AS has_other_msgstrs,
(Other.id = Current.id) AS same_on_both_sides
FROM TranslationTemplateItem AS TTI
+ JOIN POTMsgSet ON POTMsgSet.id = TTI.potmsgset
JOIN TranslationMessage AS Current ON
Current.potmsgset = TTI.potmsgset AND
Current.language = %(language)s AND
@@ -950,7 +955,9 @@
Suggestion.date_created > COALESCE(
Current.date_reviewed,
Current.date_created,
- TIMESTAMP 'epoch')
+ TIMESTAMP 'epoch') AND
+ COALESCE(Suggestion.potemplate, %(potemplate)s) =
+ %(potemplate)s
)
ORDER BY TTI.potmsgset, Current.potemplate NULLS LAST
) AS messages_with_suggestions
=== modified file 'lib/lp/translations/tests/test_pofile.py'
--- lib/lp/translations/tests/test_pofile.py 2011-02-22 01:57:30 +0000
+++ lib/lp/translations/tests/test_pofile.py 2011-02-24 11:41:50 +0000
@@ -1848,6 +1848,35 @@
self.assertEqual(0, self.pofile.newCount())
self.assertEqual(0, self.pofile.updatesCount())
+ def test_partial_translations_count_as_untranslated(self):
+ # A translation requiring plural forms is considered to be
+ # untranslated if at least one plural translation required
+ # for the given language is missing.
+ plural_potmsgset = self.factory.makePOTMsgSet(
+ self.potemplate, singular='singular-en', plural='plural-en')
+ self.factory.makeCurrentTranslationMessage(
+ pofile=self.pofile, potmsgset=plural_potmsgset,
+ translations=['sr-singular', 'sr-plural-1'])
+ self.pofile.updateStatistics()
+ self.assertEqual(0, self.pofile.translatedCount())
+ self.assertEqual(2, self.pofile.untranslatedCount())
+ self.assertEqual(0, self.pofile.newCount())
+ self.assertEqual(0, self.pofile.updatesCount())
+
+ def test_complete_translations_count_as_translated(self):
+ # A translation requiring plural forms is considered to be
+ # translated if all variants are translated.
+ plural_potmsgset = self.factory.makePOTMsgSet(
+ self.potemplate, singular='singular-en', plural='plural-en')
+ self.factory.makeCurrentTranslationMessage(
+ pofile=self.pofile, potmsgset=plural_potmsgset,
+ translations=['sr-singular', 'sr-plural-1', 'sr-plural-2'])
+ self.pofile.updateStatistics()
+ self.assertEqual(1, self.pofile.translatedCount())
+ self.assertEqual(1, self.pofile.untranslatedCount())
+ self.assertEqual(1, self.pofile.newCount())
+ self.assertEqual(0, self.pofile.updatesCount())
+
def test_empty_messages_on_this_side_count_as_untranslated(self):
# A POTMsgSet whose current TranslationMessage on this side is
# empty is counted only as untranslated, regardless of any
@@ -1863,6 +1892,25 @@
self.assertEqual(0, self.pofile.newCount())
self.assertEqual(0, self.pofile.updatesCount())
+ def test_partial_messages_on_this_side_count_as_untranslated(self):
+ # A POTMsgSet whose current TranslationMessage on this side is
+ # partially translated is considered to be untranslated, regardless
+ # of any translations it may have on the other side.
+ plural_potmsgset = self.factory.makePOTMsgSet(
+ self.potemplate, singular='singular-en', plural='plural-en')
+ self.factory.makeCurrentTranslationMessage(
+ pofile=self.pofile, potmsgset=plural_potmsgset,
+ translations=['sr-singular', 'sr-plural-1'])
+ other_message = self.factory.makeSuggestion(
+ pofile=self.pofile, potmsgset=plural_potmsgset,
+ translations=['sr-ubuntu', 'sr-ubuntu-2', 'sr-ubuntu-3'])
+ other_message.is_current_ubuntu = True
+ self.pofile.updateStatistics()
+ self.assertEqual(0, self.pofile.translatedCount())
+ self.assertEqual(2, self.pofile.untranslatedCount())
+ self.assertEqual(0, self.pofile.newCount())
+ self.assertEqual(0, self.pofile.updatesCount())
+
def test_empty_messages_on_other_side_count_as_untranslated(self):
# A POTMsgSet that's translated on this side but has an empty
# translation on the other side counts as translated on this
@@ -1882,6 +1930,29 @@
self.assertEqual(0, self.pofile.updatesCount())
self.assertEqual(0, self.pofile.currentCount())
+ def test_partial_messages_on_other_side_count_as_untranslated(self):
+ # A POTMsgSet that's translated on this side and has a different
+ # partial translation on the other side counts as translated on
+ # this side, but not equal between both sides (currentCount) or
+ # translated differently between the two sides (updatesCount).
+ # Instead, it's counted as translated on this side but not on
+ # the other (newCount).
+ plural_potmsgset = self.factory.makePOTMsgSet(
+ self.potemplate, singular='singular-en', plural='plural-en')
+ self.factory.makeCurrentTranslationMessage(
+ pofile=self.pofile, potmsgset=plural_potmsgset,
+ translations=['sr-singular', 'sr-plural1', 'sr-plural2'])
+ other_message = self.factory.makeSuggestion(
+ pofile=self.pofile, potmsgset=plural_potmsgset,
+ translations=['sr-ubuntu'])
+ other_message.is_current_ubuntu = True
+ self.pofile.updateStatistics()
+ self.assertEqual(1, self.pofile.translatedCount())
+ self.assertEqual(1, self.pofile.untranslatedCount())
+ self.assertEqual(1, self.pofile.newCount())
+ self.assertEqual(0, self.pofile.updatesCount())
+ self.assertEqual(0, self.pofile.currentCount())
+
def test_tracking_empty_messages_count_as_untranslated(self):
# An empty TranslationMessage that's current on both sides
# counts as untranslated.
@@ -1895,6 +1966,59 @@
self.assertEqual(0, self.pofile.updatesCount())
self.assertEqual(0, self.pofile.currentCount())
+ def test_tracking_partial_translations_count_as_untranslated(self):
+ # A partial Translations that's current on both sides
+ # counts as untranslated.
+ plural_potmsgset = self.factory.makePOTMsgSet(
+ self.potemplate, singular='singular-en', plural='plural-en')
+ self.factory.makeCurrentTranslationMessage(
+ pofile=self.pofile, potmsgset=plural_potmsgset,
+ translations=['sr-singular', 'sr-plural1'], current_other=True)
+ self.pofile.updateStatistics()
+ self.assertEqual(0, self.pofile.translatedCount())
+ self.assertEqual(2, self.pofile.untranslatedCount())
+ self.assertEqual(0, self.pofile.newCount())
+ self.assertEqual(0, self.pofile.updatesCount())
+ self.assertEqual(0, self.pofile.currentCount())
+
+ def makeDivergedTranslationForOtherTarget(self, for_sourcepackage):
+ """Create a translation message that is diverged for another target.
+ """
+ if for_sourcepackage:
+ ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
+ distroseries = self.factory.makeDistroSeries(distribution=ubuntu)
+ sourcepackage = self.factory.makeSourcePackage(
+ distroseries=distroseries)
+ sourcepackagename = sourcepackage.sourcepackagename
+ else:
+ distroseries = None
+ sourcepackagename = None
+ other_potemplate = self.factory.makePOTemplate(
+ distroseries=distroseries, sourcepackagename=sourcepackagename)
+ other_pofile = self.factory.makePOFile(
+ language_code=self.pofile.language.code,
+ potemplate=other_potemplate)
+ self.potmsgset.setSequence(
+ other_potemplate, self.factory.getUniqueInteger())
+ self.factory.makeCurrentTranslationMessage(
+ pofile=other_pofile, potmsgset=self.potmsgset, diverged=True)
+
+ def test_POFile_updateStatistics_diverged_message_this_side(self):
+ # Translations that are diverged for a given target do not
+ # appear in the statistical data for another target.
+ self.makeDivergedTranslationForOtherTarget(for_sourcepackage=False)
+ self.pofile.updateStatistics()
+ self.assertEqual(self.pofile.rosettaCount(), 0)
+ self.assertEqual(self.pofile.unreviewedCount(), 0)
+
+ def test_POFile_updateStatistics_diverged_message_other_side(self):
+ # Translations that are diverged for a given target do not
+ # appear in the statistical data for another target.
+ self.makeDivergedTranslationForOtherTarget(for_sourcepackage=True)
+ self.pofile.updateStatistics()
+ self.assertEqual(self.pofile.rosettaCount(), 0)
+ self.assertEqual(self.pofile.unreviewedCount(), 0)
+
class TestPOFile(TestCaseWithFactory):
"""Test PO file methods."""
=== modified file 'lib/lp/translations/utilities/rosettastats.py'
--- lib/lp/translations/utilities/rosettastats.py 2010-02-08 23:41:43 +0000
+++ lib/lp/translations/utilities/rosettastats.py 2011-02-24 11:41:50 +0000
@@ -80,16 +80,9 @@
if self.messageCount() > 0:
percent = float(value) / self.messageCount()
percent *= 100
- percent = round(percent, 2)
else:
percent = 0
- # We use float(str()) to prevent problems with some floating point
- # representations that could give us:
- # >>> x = 3.141592
- # >>> round(x, 2)
- # 3.1400000000000001
- # >>>
- return float(str(percent))
+ return percent
def translatedPercentage(self, language=None):
"""See IRosettaStats."""
@@ -114,4 +107,3 @@
def rosettaPercentage(self, language=None):
"""See IRosettaStats."""
return self.asPercentage(self.rosettaCount(language))
-
