Add Forecast Class #68
Conversation
|
I'm done changing things until I get feedback from @MatthewFlamm Edit: I've spent some more time thinking about this and committed some changes. Would like to get feedback. |
|
Breaking and unrelated changes have been reverted |
pynws/forecast.py
Outdated
| """When the forecast was last updated""" | ||
| return self.update_time | ||
|
|
||
| def get_forecast_for_time(self, when): |
There was a problem hiding this comment.
Can we have a separate method to get the value of only one layer?
There was a problem hiding this comment.
Does it make sense to allow when to be an Iterable too, like a list? A future implementation could have an additional method to automatically generate an hourly forecast for x days or hours.
There was a problem hiding this comment.
Can we have a separate method to get the value of only one layer?
One of my iterations worked that way. I will add a method for that.
There was a problem hiding this comment.
Having both makes sense to me
There was a problem hiding this comment.
Does it make sense to allow
whento be an Iterable too, like a list? A future implementation could have an additional method to automatically generate an hourly forecast forxdays or hours.
- Are you thinking that 'when' could contain a list of datetime values?
- I like the idea of generating N forecast values for hours or days. That could replace the original gridpoint hourly and daily forecasts which could eliminate a lot of code.
There was a problem hiding this comment.
- Are you thinking that 'when' could contain a list of datetime values?
Yes.
- I like the idea of generating N forecast values for hours or days. That could replace the original gridpoint hourly and daily forecasts which could eliminate a lot of code.
I think it should be a parallel option. The daily endpoint in particular has a nice human readable forecast.
There was a problem hiding this comment.
- Are you thinking that 'when' could contain a list of datetime values?
Yes.
It would probably be better to have a separate method for that because the return value would have a different shape (like the output from the next item)
- I like the idea of generating N forecast values for hours or days. That could replace the original gridpoint hourly and daily forecasts which could eliminate a lot of code.
I think it should be a parallel option. The daily endpoint in particular has a nice human readable forecast.
Fair point.
There was a problem hiding this comment.
Does it make sense to allow
whento be an Iterable too, like a list? A future implementation could have an additional method to automatically generate an hourly forecast forxdays or hours.
These have both been added (for X hours--not days)
|
@MatthewFlamm All requested changes have been made. Please let me know if there's anything else you'd like to have changed. |
|
I read through this again, and I'm happy with it. Great job and thanks! |
Added Forecast class to retrieve layer values for a given point in time
Change daily and hourly APIs to match the "all" API
Use Python relative import statements
Fix pytest warnings
Closes #66