Current FilePage.get_file_history() logic:
if not hasattr(self, '_file_revisions'):
self.site.loadimageinfo(self, history=True)
return self._file_revisionshasattr(self, '_file_revisions') will never return False because it is set on instance initialization (__init__):
self._file_revisions = {}@Mpaa Since you were the original author of these related code (d298fac), could you explain why self._file_revisions should be set on instance initialization? To me it seems counter-intuitive.
ugh ... 2 years back, I wish I could remember :-)
I also went through the review process but did not find a clue.
I guess because api.update_page() expects it for a FilePage:
if 'imageinfo' in pagedict:
assert(isinstance(page, pywikibot.FilePage))
for file_rev in pagedict['imageinfo']:
file_revision = pywikibot.page.FileInfo(file_rev)
page._file_revisions[file_revision.timestamp] = file_revisionI agree it is wrong. Maybe the best is to replace:
if not hasattr(self, '_file_revisions'):
with
if not len(self._file_revisions):
as for the other methods (probably I forgot to update get_file_history() at some point of the development of this patch).
Hmm. I see that api code has been moved to FilePage._load_file_revisions() now.
Prior to the move, close to that code we have the logic for templates:
if "templates" in pagedict:
templates = [pywikibot.Page(page.site, tl['title'])
for tl in pagedict['templates']]
if hasattr(page, "_templates"):
page._templates.extend(templates)
else:
page._templates = templatesIs it better to use this kind of logic instead? My main concern with checking len(self._file_revisions) is that, if there are in fact no file revisions on wiki, the empty result will not be cached.
Hmm. I guess len(self._file_revisions) makes sense. If it really doesn't have a file revision, it shouldn't be a file page, and all the methods of FilePage are meaningless here.
Change 333215 had a related patch set uploaded (by Zhuyifei1999):
FilePage.get_file_history(): Check for len(self._file_revisions)
Change 333291 had a related patch set uploaded (by Dalba):
page_tests.py: Add a dry test for FilePage.get_file_history
Change 333215 merged by jenkins-bot:
FilePage.get_file_history(): Check for len(self._file_revisions)
Change 333291 merged by jenkins-bot:
page_tests.py: Add a dry test for FilePage.get_file_history