[Pywikibot-commits] [Gerrit] (perf) Parse ItemPage title once - change (pywikibot/core)

jenkins-bot has submitted this change and it was merged. Change subject: (perf) Parse ItemPage title once ...................................................................... (perf) Parse ItemPage title once ItemPage may be constructed without a page title, as it is delay loaded from the wikibase_item of a page on another site. Optimise ItemPage.title() so it does not reparse the Link title every invocation. Add test case for instantiating an empty ItemPage and setting .id to a valid Qxx, and another case for altering .id highlighting a bug yet to be fixed. Split from: If7ca06adbb4b9932bba0abffc7588afcb320e934 Change-Id: I95a053a1ea17611498fcb5601c09a83d82f91fe8 --- M pywikibot/page.py M tests/wikibase_tests.py 2 files changed, 49 insertions(+), 3 deletions(-) Approvals: John Vandenberg: Looks good to me, but someone else must approve Legoktm: Looks good to me, approved jenkins-bot: Verified diff --git a/pywikibot/page.py b/pywikibot/page.py index 4675a72..7a0d1bd 100644 --- a/pywikibot/page.py +++ b/pywikibot/page.py @@ -2580,10 +2580,16 @@ self._isredir = False # Wikibase pages cannot be a redirect def title(self, **kwargs): - """ Page title. """ + """ Page title. + + If the item was instantiated without an ID, + fetch the ID and reparse the title. + """ if self.namespace() == 0: - self._link._text = self.getID() - del self._link._title + self.getID() + if self._link._text != self.id: + self._link._text = self.id + del self._link._title return Page(self).title(**kwargs) @deprecated("_defined_by") diff --git a/tests/wikibase_tests.py b/tests/wikibase_tests.py index 4059c4a..aca60b3 100644 --- a/tests/wikibase_tests.py +++ b/tests/wikibase_tests.py @@ -167,6 +167,46 @@ item.get() self.assertEquals(hasattr(item, '_content'), True) + def test_load_item_set_id(self): + """Test setting item.id attribute on empty item.""" + item = pywikibot.ItemPage(wikidata, '-1') + self.assertEquals(item._link._title, '-1') + item.id = 'Q60' + self.assertEquals(hasattr(item, '_content'), False) + self.assertEquals(item.getID(), 'Q60') + self.assertEquals(hasattr(item, '_content'), False) + item.get() + self.assertEquals(hasattr(item, '_content'), True) + self.assertTrue('en' in item.labels) + self.assertEquals(item.labels['en'], 'New York City') + self.assertEquals(item.title(), 'Q60') + + def test_reuse_item_set_id(self): + """ + Test modifying item.id attribute. + + Some scripts are using item.id = 'Q60' semantics, which does work + but modifying item.id does not currently work, and this test + highlights that it breaks silently. + """ + item = pywikibot.ItemPage(wikidata, 'Q60') + item.get() + self.assertEquals(item.labels['en'], 'New York City') + + # When the id attribute is modified, the ItemPage goes into + # an inconsistent state. + item.id = 'Q5296' + # The title is updated correctly + self.assertEquals(item.title(), 'Q5296') + + # This del has no effect on the test; it is here to demonstrate that + # it doesnt help to clear this piece of saved state. + del item._content + # The labels are not updated; assertion showing undesirable behaviour: + self.assertEquals(item.labels['en'], 'New York City') + # TODO: This is the assertion that this test should be using: + #self.assertTrue(item.labels['en'].lower().endswith('main page')) + def test_empty_item(self): # should not raise an error as the constructor only requires # the site parameter, with the title parameter defaulted to None -- To view, visit https://gerrit.wikimedia.org/r/137562 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I95a053a1ea17611498fcb5601c09a83d82f91fe8 Gerrit-PatchSet: 4 Gerrit-Project: pywikibot/core Gerrit-Branch: master Gerrit-Owner: John Vandenberg &lt;jayvdb(a)gmail.com&gt; Gerrit-Reviewer: Bthfan &lt;bth-fan(a)mcsmurf.de&gt; Gerrit-Reviewer: John Vandenberg &lt;jayvdb(a)gmail.com&gt; Gerrit-Reviewer: Ladsgroup &lt;ladsgroup(a)gmail.com&gt; Gerrit-Reviewer: Legoktm &lt;legoktm.wikipedia(a)gmail.com&gt; Gerrit-Reviewer: Merlijn van Deen &lt;valhallasw(a)arctus.nl&gt; Gerrit-Reviewer: Multichill &lt;maarten(a)mdammers.nl&gt; Gerrit-Reviewer: Ricordisamoa &lt;ricordisamoa(a)openmailbox.org&gt; Gerrit-Reviewer: Underlying lk &lt;a375070(a)drdrb.net&gt; Gerrit-Reviewer: jenkins-bot <>