[Pywikibot-commits] [Gerrit] ...core[master]: [IMPR] improve self.toJSON() methods in page.__init__.py

jenkins-bot has submitted this change. ( https://gerrit.wikimedia.org/r/c/pywikibot/core/+/642564 ) Change subject: [IMPR] improve self.toJSON() methods in page.__init__.py ...................................................................... [IMPR] improve self.toJSON() methods in page.__init__.py "Keys views are set-like since their entries are unique and hashable." Use this fact to reduce code complexity. In AliasesDict.toJSON(): - consider also keys in self when making diff. This seems to make more sense and is aligned with LanguageDict.toJSON() method. Enriched text cases to consider all possible combinations in: set(A) - set(B) set(A) & set(B) set(B) - set(A) Change-Id: I906046ffb184605e6a31035b6c44af333973a249 --- M pywikibot/page/__init__.py M tests/wikibase_tests.py 2 files changed, 56 insertions(+), 42 deletions(-) Approvals: Matěj Suchánek: Looks good to me, but someone else must approve Xqt: Looks good to me, approved jenkins-bot: Verified diff --git a/pywikibot/page/__init__.py b/pywikibot/page/__init__.py index 9979bdc..64e3d54 100644 --- a/pywikibot/page/__init__.py +++ b/pywikibot/page/__init__.py @@ -3421,17 +3421,13 @@ def toJSON(self, diffto=None): data = {} - if diffto: - for key in diffto: - if key not in self: - data[key] = {'language': key, 'value': ''} - elif self[key] != diffto[key]['value']: - data[key] = {'language': key, 'value': self[key]} - for key in self: - if key not in diffto: - data[key] = {'language': key, 'value': self[key]} - else: - for key in self: + diffto = diffto or {} + for key in diffto.keys() - self.keys(): + data[key] = {'language': key, 'value': ''} + for key in self.keys() - diffto.keys(): + data[key] = {'language': key, 'value': self[key]} + for key in self.keys() & diffto.keys(): + if self[key] != diffto[key]['value']: data[key] = {'language': key, 'value': self[key]} return data @@ -3499,20 +3495,18 @@ def toJSON(self, diffto=None): data = {} - if diffto: - for lang, strings in diffto.items(): - if len(self.get(lang, [])) > 0: - if tuple(sorted(val['value'] for val in strings)) != tuple( - sorted(self[lang])): - data[lang] = [{'language': lang, 'value': i} - for i in self[lang]] - else: - data[lang] = [ - {'language': lang, 'value': i['value'], 'remove': ''} - for i in strings] - else: - for lang, values in self.items(): - data[lang] = [{'language': lang, 'value': i} for i in values] + diffto = diffto or {} + for lang in diffto.keys() & self.keys(): + if (sorted(val['value'] for val in diffto[lang]) + != sorted(self[lang])): + data[lang] = [{'language': lang, 'value': i} + for i in self[lang]] + for lang in diffto.keys() - self.keys(): + data[lang] = [ + {'language': lang, 'value': i['value'], 'remove': ''} + for i in diffto[lang]] + for lang in self.keys() - diffto.keys(): + data[lang] = [{'language': lang, 'value': i} for i in self[lang]] return data diff --git a/tests/wikibase_tests.py b/tests/wikibase_tests.py index 13a028c..e485ea9 100644 --- a/tests/wikibase_tests.py +++ b/tests/wikibase_tests.py @@ -1808,14 +1808,14 @@ """Setup tests.""" super().setUp() self.site = self.get_site() - self.lang_out = {'en': 'foo'} + self.lang_out = {'en': 'foo', 'zh': 'bar'} def test_init(self): """Test LanguageDict initializer.""" ld = LanguageDict() self.assertLength(ld, 0) ld = LanguageDict(self.lang_out) - self.assertLength(ld, 1) + self.assertLength(ld, 2) def test_setitem(self): """Test LanguageDict.__setitem__ metamethod.""" @@ -1835,12 +1835,16 @@ """Test LanguageDict.__delitem__ metamethod.""" ld = LanguageDict(self.lang_out) ld.pop(self.site) + ld.pop('zh') self.assertNotIn('en', ld) + self.assertNotIn('zh', ld) self.assertLength(ld, 0) def test_fromJSON(self): """Test LanguageDict.fromJSON method.""" - ld = LanguageDict.fromJSON({'en': {'language': 'en', 'value': 'foo'}}) + ld = LanguageDict.fromJSON( + {'en': {'language': 'en', 'value': 'foo'}, + 'zh': {'language': 'zh', 'value': 'bar'}}) self.assertIsInstance(ld, LanguageDict) self.assertEqual(ld, LanguageDict(self.lang_out)) @@ -1850,24 +1854,27 @@ self.assertEqual(ld.toJSON(), {}) ld = LanguageDict(self.lang_out) self.assertEqual( - ld.toJSON(), {'en': {'language': 'en', 'value': 'foo'}}) + ld.toJSON(), {'en': {'language': 'en', 'value': 'foo'}, + 'zh': {'language': 'zh', 'value': 'bar'}}) def test_toJSON_diffto(self): """Test LanguageDict.toJSON method.""" - ld = LanguageDict({'de': 'foo'}) + ld = LanguageDict({'de': 'foo', 'zh': 'bar'}) diffto = { 'de': {'language': 'de', 'value': 'bar'}, 'en': {'language': 'en', 'value': 'foo'}} self.assertEqual( ld.toJSON(diffto=diffto), {'de': {'language': 'de', 'value': 'foo'}, - 'en': {'language': 'en', 'value': ''}}) + 'en': {'language': 'en', 'value': ''}, + 'zh': {'language': 'zh', 'value': 'bar'}}) def test_normalizeData(self): """Test LanguageDict.normalizeData method.""" self.assertEqual( LanguageDict.normalizeData(self.lang_out), - {'en': {'language': 'en', 'value': 'foo'}}) + {'en': {'language': 'en', 'value': 'foo'}, + 'zh': {'language': 'zh', 'value': 'bar'}}) def test_new_empty(self): """Test that new_empty method returns empty collection.""" @@ -1889,19 +1896,21 @@ """Setup tests.""" super().setUp() self.site = self.get_site() - self.lang_out = {'en': ['foo', 'bar']} + self.lang_out = {'en': ['foo', 'bar'], + 'zh': ['foo', 'bar']} def test_init(self): """Test AliasesDict initializer.""" ad = AliasesDict() self.assertLength(ad, 0) ad = AliasesDict(self.lang_out) - self.assertLength(ad, 1) + self.assertLength(ad, 2) def test_setitem(self): """Test AliasesDict.__setitem__ metamethod.""" ad = AliasesDict(self.lang_out) self.assertIn('en', ad) + self.assertIn('zh', ad) ad[self.site] = ['baz'] self.assertIn('en', ad) @@ -1916,14 +1925,19 @@ """Test AliasesDict.__delitem__ metamethod.""" ad = AliasesDict(self.lang_out) ad.pop(self.site) + ad.pop('zh') self.assertNotIn('en', ad) + self.assertNotIn('zh', ad) self.assertLength(ad, 0) def test_fromJSON(self): """Test AliasesDict.fromJSON method.""" - ad = AliasesDict.fromJSON({'en': [ - {'language': 'en', 'value': 'foo'}, - {'language': 'en', 'value': 'bar'}]}) + ad = AliasesDict.fromJSON( + {'en': [{'language': 'en', 'value': 'foo'}, + {'language': 'en', 'value': 'bar'}], + 'zh': [{'language': 'zh', 'value': 'foo'}, + {'language': 'zh', 'value': 'bar'}], + }) self.assertIsInstance(ad, AliasesDict) self.assertEqual(ad, AliasesDict(self.lang_out)) @@ -1932,10 +1946,13 @@ ad = AliasesDict() self.assertEqual(ad.toJSON(), {}) ad = AliasesDict(self.lang_out) - self.assertEqual(ad.toJSON(), {'en': [ - {'language': 'en', 'value': 'foo'}, - {'language': 'en', 'value': 'bar'}, - ]}) + self.assertEqual( + ad.toJSON(), + {'en': [{'language': 'en', 'value': 'foo'}, + {'language': 'en', 'value': 'bar'}], + 'zh': [{'language': 'zh', 'value': 'foo'}, + {'language': 'zh', 'value': 'bar'}], + }) def test_toJSON_diffto(self): """Test AliasesDict.toJSON method.""" @@ -1954,7 +1971,10 @@ {'de': [{'language': 'de', 'value': 'foo', 'remove': ''}, {'language': 'de', 'value': 'bar', 'remove': ''}], 'en': [{'language': 'en', 'value': 'foo'}, - {'language': 'en', 'value': 'bar'}]}) + {'language': 'en', 'value': 'bar'}], + 'zh': [{'language': 'zh', 'value': 'foo'}, + {'language': 'zh', 'value': 'bar'}] + }) def test_normalizeData(self): """Test AliasesDict.normalizeData method.""" -- To view, visit https://gerrit.wikimedia.org/r/c/pywikibot/core/+/642564 To unsubscribe, or for help writing mail filters, visit https://gerrit.wikimedia.org/r/settings Gerrit-Project: pywikibot/core Gerrit-Branch: master Gerrit-Change-Id: I906046ffb184605e6a31035b6c44af333973a249 Gerrit-Change-Number: 642564 Gerrit-PatchSet: 5 Gerrit-Owner: Mpaa <mpaa.wiki(a)gmail.com> Gerrit-Reviewer: Matěj Suchánek <matejsuchanek97(a)gmail.com> Gerrit-Reviewer: Xqt <info(a)gno.de> Gerrit-Reviewer: jenkins-bot Gerrit-MessageType: merged