Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the Flickr, iNaturalist, and YouTube bots to improve robustness and maintainability. Key updates include replacing assert statements with explicit conditional checks in hook methods, moving the taxa cache from class to instance level in INaturalistBot, and cleaning up docstrings and unused imports. Feedback identifies that the manual template parsing introduced in flickr.py is redundant and suggests reverting to the existing retrieve_template_data helper method for consistency and maintainability.
src/wikibots/flickr.py
Outdated
| self.parse_wikicode() | ||
| assert self.wiki_properties.wikicode | ||
|
|
||
| templates = [ | ||
| w | ||
| for w in self.wiki_properties.wikicode.filter_templates() | ||
| if w.name.strip() == "FlickreviewR" | ||
| ] | ||
| if not templates: | ||
| warning("No FlickreviewR template found") | ||
| self.redis.set(self.wiki_properties.redis_key, 1) | ||
| return None | ||
|
|
||
| t = templates[0] | ||
| review_status = str(t.get("status").value).strip() if t.has("status") else None | ||
| if review_status != "pass": | ||
| warning(f"Skipping: FlickreviewR status is {review_status!r}, not 'pass'") | ||
| self.redis.set(self.wiki_properties.redis_key, 1) | ||
| return None | ||
|
|
||
| flickr_url = self.retrieve_template_data(["FlickreviewR"], ["sourceurl"]) | ||
| flickr_url = ( | ||
| str(t.get("sourceurl").value).strip() if t.has("sourceurl") else None | ||
| ) | ||
| if flickr_url is None: | ||
| warning("FlickreviewR template is missing sourceurl") | ||
| self.redis.set(self.wiki_properties.redis_key, 1) | ||
| return None |
There was a problem hiding this comment.
The manual parsing of the FlickreviewR template is redundant because the BaseBot class already provides the retrieve_template_data helper method. This method handles wikicode parsing, template filtering, parameter extraction, logging, and Redis state management in a consistent way. Reverting to the helper method improves maintainability and reduces code duplication, as seen in other bot implementations like inaturalist.py and youtube.py.
| self.parse_wikicode() | |
| assert self.wiki_properties.wikicode | |
| templates = [ | |
| w | |
| for w in self.wiki_properties.wikicode.filter_templates() | |
| if w.name.strip() == "FlickreviewR" | |
| ] | |
| if not templates: | |
| warning("No FlickreviewR template found") | |
| self.redis.set(self.wiki_properties.redis_key, 1) | |
| return None | |
| t = templates[0] | |
| review_status = str(t.get("status").value).strip() if t.has("status") else None | |
| if review_status != "pass": | |
| warning(f"Skipping: FlickreviewR status is {review_status!r}, not 'pass'") | |
| self.redis.set(self.wiki_properties.redis_key, 1) | |
| return None | |
| flickr_url = self.retrieve_template_data(["FlickreviewR"], ["sourceurl"]) | |
| flickr_url = ( | |
| str(t.get("sourceurl").value).strip() if t.has("sourceurl") else None | |
| ) | |
| if flickr_url is None: | |
| warning("FlickreviewR template is missing sourceurl") | |
| self.redis.set(self.wiki_properties.redis_key, 1) | |
| return None | |
| review_status = self.retrieve_template_data(["FlickreviewR"], ["status"]) | |
| if review_status != "pass": | |
| warning(f"Skipping: FlickreviewR status is {review_status!r}, not 'pass'") | |
| self.redis.set(self.wiki_properties.redis_key, 1) | |
| return None | |
| flickr_url = self.retrieve_template_data(["FlickreviewR"], ["sourceurl"]) | |
| if flickr_url is None: | |
| return None |
There was a problem hiding this comment.
Reverted. The helper already handles caching, logging, and Redis — no reason to inline it.
No description provided.