Fix postmerge errors #35
Conversation
create_infotexts.py
Outdated
| """" | ||
| Check if there are any content cats added and add generic content category to image. | ||
|
|
||
| Populate self.content_cat with commons category, if present. | ||
| """ | ||
| if not self.content_cats: | ||
| if len(self.content_cats) == 0: |
There was a problem hiding this comment.
Why this change? The previous one should catch both empty list and None
There was a problem hiding this comment.
I changed because I tried this:
In [25]: l = []
In [26]: l == True
Out[26]: False
In [27]: l is True
Out[27]: False
In [28]: l == False
Out[28]: False
In [29]: l is False
Out[29]: False
However, quirky enough the following works:
In [30]: if l:
print("l is True?")
else:
print("nah, l is False.")
nah, l is False.
There was a problem hiding this comment.
That is because if doesn't just test for True/False but (in this case also interprets None,[],{},0 and likely other empty elements as False.
The only one of these that might be very dangerous is 0. I.e. if you are evaluating numbers and num=0 is a valid number then you cannot use if num to guard agains num is None.
But that is not the case here so if not self.content_cats: should be used.
create_infotexts.py
Outdated
| @@ -371,7 +371,11 @@ def process_depicted_place(self, place_string, places_mapping, desc_string): | |||
| place_as_wikitext = "" | |||
|
|
|||
| if place_string: | |||
| if place_string in places_mapping: | |||
| # Don't forget to add the commons categories, even though only wikidata is used in depicted people field | |||
| if places_mapping[place_string].get('commonscat'): | |||
There was a problem hiding this comment.
This re-jig doesn't work.
In the previous version we first checked that place_string was in places_mapping. If this was the case we then also checked for a commonscat. Here places_mapping[place_string] is likely to throw a keyError.
To fix re-add if place_string in places_mapping: before this (see also next comment).
create_infotexts.py
Outdated
| if places_mapping[place_string].get('commonscat'): | ||
| self.content_cats.append(places_mapping[place_string]["commonscat"]) | ||
|
|
||
| if place_string.lower() in [place.lower() for place in places_mapping.keys()]: |
There was a problem hiding this comment.
Is there really a mixture of upper/lower in both the mapping and the in-data?
If so I would standardise the mapping data (just make them all lower). Then in the if place_string in places_mapping above (see previous comment) simply change place_string to place_string.lower() (and drop this if).
create_infotexts.py
Outdated
| else: | ||
| place_matches = [] | ||
| for place in places_mapping: | ||
| if place.lower() in desc_string.lower(): | ||
| place_matches.append(place) | ||
| # Don't forget to add the commons categories if present. | ||
| if places_mapping[place].get('commonscat'): |
There was a problem hiding this comment.
Task: https://phabricator.wikimedia.org/T164715