Created price.py to incorporate OpenEI API integration. #162
Conversation
|
When I run lint.sh locally, I see that 89 files are unchanged but 1 file failed to reformat, zeus/monitor/test_carbon.py. I have fully updated black, ruff, and pyright with no changes. |
|
I fresh-reinstalled Zeus on an empty virtual environment and I see zero reformats or linting issues. I use miniconda: conda deactivate && conda remove -n zeus --all -y && conda create -n zeus python=3.9 -y && conda activate zeus && pip install -e '.[dev]'So I think your setup is broken. The PR shows 32 changed files (mostly formatting) and the CI also says it'll end up reformatting 32 files, which are probably reverting your changes. |
|
I incorporated your changes and it appears to have worked. |
There was a problem hiding this comment.
I took a high-level look and it's in good shape! Have you ran this locally and does it work?
If so, please add automated tests using pytest. After tests are added, I'll look at everything again in more detail. Ref: https://github.com/ml-energy/zeus/tree/master/tests/monitor
In general, I would select multiple electricity providers from OpenEI (optimize for coverage of different characteristics of electricity providers), query OpenEI, and put the resulting API response inside the test directory (ilke carbon_history_files/). With those, tests can be run multiple times without having to query the API every time.
zeus/monitor/price.py
Outdated
| raise | ||
|
|
||
|
|
||
| def get_time_info(): |
There was a problem hiding this comment.
Please make sure all functions (arguments and return values) are typed correctly.
zeus/monitor/price.py
Outdated
| logger = get_logger(__name__) | ||
|
|
||
|
|
||
| def get_ip_lat_long() -> tuple[float, float]: |
There was a problem hiding this comment.
Don't duplicate existing functions. Move it into utils and share.
zeus/monitor/price.py
Outdated
| """Abstract class for implementing ways to fetch electricity price.""" | ||
|
|
||
| @abc.abstractmethod | ||
| def get_current_electricity_price(self) -> dict: |
There was a problem hiding this comment.
dict is an insufficient type. What is the type of the key and value?
Co-authored-by: Jae-Won Chung <[email protected]>
…into price-integration
The OpenEI API provides electricity pricing, similar to carbon.py for carbon intensity. I query it once because these rates change only once a year.