Add or update type hints #82
Conversation
There was a problem hiding this comment.
Looking good!
Please pull in changes from master branch so the typing CI runs here. Now that #79 is merged.
|
The use of return Types isn't consistent in this PR, but I'm not sure how important it is to indicate a |
Co-authored-by: MatthewFlamm <39341281+MatthewFlamm@users.noreply.github.com>
This is just a starting point. Please feel free to commit fixes directly to this PR. |
|
Many methods have been annotated. There are some remaining, but I'm done with this for now. Feel free to continue or merge as-is if you approve. |
…o indicate expected read/write usage
…Dict and List to indicate expected read/write usage
| @@ -87,48 +111,60 @@ async def get_detailed_forecast(self: Nws) -> DetailedForecast: | |||
| """ | |||
| if self.wfo is None: | |||
| await self.get_points() | |||
| if self.wfo is None or self.x is None or self.y is None: | |||
There was a problem hiding this comment.
Either this should move into get_points, or it should be inside the prior if statement, which is expanded to check for self.x and self.y, or both.
There was a problem hiding this comment.
Without those checks, mypy has the following complaints.
pynws\nws.py:117: error: Argument 1 to "raw_detailed_forecast" has incompatible type "Optional[str]"; expected "str"
pynws\nws.py:117: error: Argument 2 to "raw_detailed_forecast" has incompatible type "Optional[int]"; expected "int"
pynws\nws.py:117: error: Argument 3 to "raw_detailed_forecast" has incompatible type "Optional[int]"; expected "int"
I could use cast to remove the Optional in the arguments to raw_detailed_forecast instead if you prefer. That would look like this:
if self.wfo is None:
await self.get_points()
raw_forecast = await raw_detailed_forecast(
cast(str, self.wfo),
cast(int, self.x),
cast(int, self.y),
self.session,
self.userid,
)
return DetailedForecast(raw_forecast["properties"])
One more thing, get_points silently fails when retrieving from API fails.
data = await raw_points(*self.latlon, self.session, self.userid)
properties = data.get("properties")
if properties:
self.wfo = properties.get("cwa")
self.x = properties.get("gridX")
self.y = properties.get("gridY")
self.forecast_zone = properties.get("forecastZone").split("/")[-1]
self.county_zone = properties.get("county").split("/")[-1]
self.fire_weather_zone = properties.get("fireWeatherZone").split("/")[-1]
return properties
| """Return daily forecast from grid.""" | ||
| if self.wfo is None: | ||
| await self.get_points() | ||
| if self.wfo is None or self.x is None or self.y is None: |
| """Return hourly forecast from grid.""" | ||
| if self.wfo is None: | ||
| await self.get_points() | ||
| if self.wfo is None or self.x is None or self.y is None: |
| """Returns alerts dict for forecast zone.""" | ||
| if self.forecast_zone is None: | ||
| await self.get_points() | ||
| if self.forecast_zone is None: |
There was a problem hiding this comment.
should the zone checks also move inside self.get_points?
|
My only outstanding comments are the ones about checking |
mypy passes, but some of the type hints might needs to be adjusted.