Add test to PVForecast #174
Conversation
Added documentation. Beware mostly generated by ChatGPT. Signed-off-by: Bobby Noelte <[email protected]>
|
When we change to fastapi, we will use pydantic. Maybe we could replace the jsonschema with pydantic here. |
b0661
force-pushed
the
pr_pv_forecast_on_main
branch
4 times, most recently
from
47f08d0
to
93db87b
Compare
Switched to pydantic. |
There was a problem hiding this comment.
I think currently only the url is used to fill PVForecast with data (/pvforecast). So to me the interface seems complicated supporting data (dict), filepath and url. If we want to support filepath and raw data input to the class maybe we should abstract filepath, url into PVForecastFile, PVForecastURL to do the io/network stuff (maybe one more abstraction layer for the caching) and then have the base class with all the logic operating directly on the raw data (e.g. process_data does not require any input, that's not already defined in base constructor).
| self.meta = {} | ||
| self.forecast_data = [] | ||
| self.cache_dir = cache_dir | ||
| self.prediction_hours = prediction_hours | ||
| self.cache_files = CacheFileStore() |
There was a problem hiding this comment.
For retrieving forecast data could we maybe use lru_cache from functools?
There was a problem hiding this comment.
What about cachetools TTLCache? Seems to be what is intended in the current code.
There was a problem hiding this comment.
Looked at cachetools and filecache and some other. They were missing some aspects I wanted to have. Will stay with CacheFileStore but added a function decorator cache_in_file() to make it quite easy to cache function return values.
@cache_in_file(mode="wb+")
def load_data_from_url_with_caching(self, url: str) -> dict:
response = requests.get(url)
data = response.json()
logger.debug(f"Data fetched from URL `{url} and cached.")
return data
data = pv_forecast.load_data_from_url_with_caching(url, with_ttl="2 days")
You are right. Doing this would radically change the interface. I would do that in a following PR if this one is accepted. |
b0661
force-pushed
the
pr_pv_forecast_on_main
branch
2 times, most recently
from
4c99bcf
to
d9cdf3a
Compare
The `CacheFileStore` class is a singleton-based, thread-safe key-value store for managing temporary file objects, allowing the creation, retrieval, and management of cache files. The utility modules offer a flexible logging setup (`get_logger`) and utilities to handle different date-time formats (`to_datetime`, `to_timestamp`). - Cache files are automatically valid for the the current date unless specified otherwise. This is to mimic the current behaviour used in several classes. - The logger supports rotating log files to prevent excessive log file size. - The `to_datetime` and `to_timestamp`functions support a wide variety of input types and formats. They provide the time conversion that is e.g. used in PVForecast. Signed-off-by: Bobby Noelte <[email protected]>
Improvements for testing of PVForecast - Use common utility functions to allow for general testing at one spot. - to_datetime - CacheFileStore - Use logging instead of print to easily capture in testing. - Add validation of the json schema for Akkudoktor PV forecast data. - Allow to create an empty PVForecast instance as base instance for testing. - Make process_data() complete for filling a PVForecast instance for testing. - Normalize forecast datetime to timezone of system given in loaded data. - Do not print report but provide report for test checks. - Get rid of cache file path using the CachFileStore to automate cache file usage. - Improved module documentation. Signed-off-by: Bobby Noelte <[email protected]>
- Add test for PVForecast - Add test for CacheFileStore in the new cachefilestore module - Add test for to_datetime in the new datetimeutil module - Add test for get_logger in the new logutil module Signed-off-by: Bobby Noelte <[email protected]>
b0661
force-pushed
the
pr_pv_forecast_on_main
branch
from
d9cdf3a
to
c29c222
Compare
|
Please re-run all tests, code changed quite a lot |
1 similar comment
|
Please re-run all tests, code changed quite a lot |
Tests were added for PVForecast
To improve testability several steps were done. Coverage currently is around 85%.