chore: more weather pathfinder improvements

[tiftran]

References

Description

Shrunk down the pathfinder list by adding functions to remove the locality suffix, and normalizing the city as an option to try. I left some in the list like Đồng Nại": "Dong Nai", since when normalized it gives ong Nai

PR Review Checklist

Put an x in the boxes that apply

• This PR conforms to the Contribution Guidelines
• The PR title starts with the JIRA issue reference, format example [DISCO-####], and has the same title (if applicable)
• [load test: (abort|skip|warn)] keywords are applied to the last commit message (if applicable)
• Documentation has been updated (if applicable)
• Functional and performance test coverage has been expanded and maintained (if applicable)

[ncloudioj]

Hmm, sure this is beneficial? I can think of "Mexico City", "Panama City", "New York City", "Salt Lake City", "Oklahoma City", "Quebec City", "Guatemala City", "Ho Chi Minh City", etc.

[tiftran]

So I did test out "New York City", "Quebec City" and they were okay, but you're right the others may or may not work. There are a couple of cities in South America that will benefit. Given that I think it would make sense to have the suffix removed as another city name to try if the original fails.

[ncloudioj]

Ah, I see. If this doesn't regress other locations, we can give it a try.

You can compile this regex (re.compile()) as a constant so it won't be compiled on the fly for each removal.

[tiftran]

k so i now have it so that we try the original, normalized, then without the city/municipality suffix

[ncloudioj]

Did you want to match one and more whitespace before "city"?

Suggested change

	LOCALITY_SUFFIX_PATTERN = re.compile(r"\s*(city|municipality)$", re.IGNORECASE)
	LOCALITY_SUFFIX_PATTERN: re.Pattern = re.compile(r"\s+(city|municipality)$", re.IGNORECASE)

[ncloudioj]

Suggested change

	return LOCALITY_SUFFIX_PATTERN.sub("", city).strip()
	return LOCALITY_SUFFIX_PATTERN.sub("", city)

[ncloudioj]

Having to do all the following upfront for every location can be quite wasteful, in particular, for those locations that don't require special handling. Let's make normalization optional and lazy.

# define this at the module level
CITY_NAME_NORMALIZERS: Callable[[Str], Str] = [
    lambda a: a,
    normalize_string,
    remove_locality_suffix,
]

async def explore(...):
    ...
    
    # if city is None, just return
    if geolocation.city is None:
        return None, is_skipped

    city: str = typing.cast(str, geolocation.city)  # tell the type checker it's truly not None 
    # map is lazy, so items of `cities` would only be evaluated one by one if needed 
    cities = map(lambda fn: fn(city), CITY_NAME_NORMALIZERS)
    explored_cities: list[str] = []  # store the explored cities to avoid duplicates

    for region in compass(weather_context.geolocation):
        for city in cities:
            if city in explored_cities:
                continue
            else:
                explored_cities.append(city)
            
             # probe the location as usual     

[tiftran]

oh that's fancy

[ncloudioj]

Since remove_locality_suffix() is already handled in explore(), do we still need it here? Doing the suffix removal in both places seems pretty hard to follow to me. Is there a way for us to not do suffix removing here?

[tiftran]

ah yup, you're right

[ncloudioj]

Hmm, is this a mis-kill? CITY_NAME_CORRECTION_MAPPING is still in use, eh?

[ncloudioj]

Looks good, r+wc.