This repository has been archived by the owner on Oct 23, 2019. It is now read-only.
Add French-language weather support 🇫🇷 #323
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This addresses snoonetIRC#271 The Weather Underground API supports specifying a return language, so the translation work locally is mostly swapping a few labels. While I like to think I know a little French, I relied entirely on Google Translate for this. For the sake of completeness though, I also translated as many error messages as I could. Also, at the prompting of my IDE, I added type information to doc-strings where it was missing. Finally, the global `location_cache` was being kept as a list of key-value pairs and converted to a dict every time it was checked. Presumably there was a historical reason for this, but for now it seems more appropriate to just make it a dict.
2 × Unnecessary "elif" after "return"
linuxdaemon
reviewed
Dec 26, 2018
| return 'Unknown Error.' | ||
| if self._status == 'INVALID_REQUEST': | ||
| return 'Invalid Request.' | ||
| return repr(self._status) |
There was a problem hiding this comment.
Please don't override __str__ on an exception subclass to set the text, use super().__init__("exception text") to do it. This is consistent with the rest of the exception API.
linuxdaemon
suggested changes
Dec 26, 2018
| for row in db.execute(table.select()): | ||
| nick = row["nick"] | ||
| location = row["loc"] | ||
| location_cache.append((nick, location)) | ||
| location_cache[nick] = location |
There was a problem hiding this comment.
Clear the cache before reloading it, as you could have old user data still there.
| @@ -149,7 +178,7 @@ def weather(text, reply, db, nick, notice_doc): | |||
|
|
|||
| forecast = response["forecast"]["simpleforecast"]["forecastday"] | |||
| if not forecast: | |||
| return "Unable to retrieve forecast data." | |||
| return 'Unable to retrieve forecast data.' | |||
There was a problem hiding this comment.
Strings shown to a user should be enclosed in double-quotes.
| # 'oui' is a pun, see https://github.com/snoonetIRC/CloudBot/issues/271 | ||
| @hook.command('météo', 'meteo', 'oui', autohelp=False) | ||
| def meteo(text, reply, db, nick, notice_doc): | ||
| """<lieu> - Quel temps fait-il à <lieu>? |
There was a problem hiding this comment.
Optional parameters should be enclosed in square-brackets [lieu]
| return e.en_francais() | ||
| if not isinstance(weather_data, dict): | ||
| if weather_data == 'Unable to retrieve forecast data.': | ||
| weather_data = 'Impossible de récupérer les données météorologiques.' |
| else: | ||
| location = location[0] | ||
| return location | ||
| class APIKeyMissing(Exception): |
There was a problem hiding this comment.
Please group class declarations together above hook functions in the file.
|
Please address the relevant review comments |
linuxdaemon
added
the
blocked
|
This is also blocked for the time being, as the weather plugin will need to be rewritten entirely due to weather underground removing their API |
linuxdaemon
removed
the
blocked
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This addresses #271
Apparently they are ending support for the API and no longer providing API keys.
According to the documentation this should work though.
Since the Weather Underground API supports specifying a return language, the translation work locally is mostly swapping a few labels. While I like to think I know a little French, I relied entirely on Google Translate for this.
For the sake of completeness though, I also translated as many error messages as I could.
Also, at the prompting of my IDE, I added type information to doc-strings where it was missing.
Finally, the global
location_cachewas being kept as a list of key-value pairs and converted to a dict every time it was checked. Presumably there was a historical reason for this, but for now it seems more appropriate to just make it a dict.