-
Notifications
You must be signed in to change notification settings - Fork 1k
Migrated pricing functionality to v4 #8424
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Migrated pricing functionality to v4 #8424
Conversation
VIKTORVAV99
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some initial feedback. Realtime works good but historical data is currently not working.
| if params: | ||
| url += "?" + "&".join(params) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should return the params as a dict, then we can pass it directly to requests.
| def _derive_interval_from_datetime(target_datetime: datetime) -> str: | ||
| """ | ||
| Derive appropriate interval based on time difference from target datetime. | ||
| Args: | ||
| target_datetime: The target datetime to compare against current time | ||
| Returns: | ||
| Appropriate interval string for the API | ||
| """ | ||
| delta = datetime.now(tz=timezone.utc) - target_datetime.replace(tzinfo=timezone.utc) | ||
| days = delta.days | ||
|
|
||
| if delta <= timedelta(hours=1): | ||
| return "5m" | ||
| elif delta <= timedelta(days=1): | ||
| return "1h" | ||
| elif days <= 7: | ||
| return "1d" | ||
| elif days <= 31: | ||
| return "7d" | ||
| elif days <= 92: | ||
| return "1M" | ||
| elif days <= 184: | ||
| return "3M" | ||
| elif days <= 366: | ||
| return "1y" | ||
| else: | ||
| return "fy" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not derive the interval, we should always use the lowest granularity available for each kind. Price should be 5 min for example.
| zone_key=zone_key, | ||
| session=session, | ||
| target_datetime=target_datetime, | ||
| kind="power", | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I run this: poetry run test_parser "AU-WA" --target_datetime="2022-01-01" this fails with:
Traceback (most recent call last):
File "<string>", line 1, in <module>
File "/Users/viktor/Repos/electricitymaps-contrib/.venv/lib/python3.10/site-packages/click/core.py", line 1161, in __call__
return self.main(*args, **kwargs)
File "/Users/viktor/Repos/electricitymaps-contrib/.venv/lib/python3.10/site-packages/click/core.py", line 1082, in main
rv = self.invoke(ctx)
File "/Users/viktor/Repos/electricitymaps-contrib/.venv/lib/python3.10/site-packages/click/core.py", line 1443, in invoke
return ctx.invoke(self.callback, **ctx.params)
File "/Users/viktor/Repos/electricitymaps-contrib/.venv/lib/python3.10/site-packages/click/core.py", line 788, in invoke
return __callback(*args, **kwargs)
File "/Users/viktor/Repos/electricitymaps-contrib/test_parser.py", line 74, in test_parser
res = parser(*args, target_datetime=parsed_target_datetime, logger=logger)
File "/Users/viktor/Repos/electricitymaps-contrib/electricitymap/contrib/parsers/lib/config.py", line 77, in wrapped_f
result = f(*args, **kwargs)
File "/Users/viktor/Repos/electricitymaps-contrib/electricitymap/contrib/parsers/OPENNEM.py", line 395, in fetch_production
datasets = fetch_datasets(
File "/Users/viktor/Repos/electricitymaps-contrib/electricitymap/contrib/parsers/OPENNEM.py", line 180, in fetch_datasets
response.raise_for_status()
File "/Users/viktor/Repos/electricitymaps-contrib/.venv/lib/python3.10/site-packages/requests/models.py", line 1024, in raise_for_status
raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 416 Client Error: Range Not Satisfiable for url: https://api.openelectricity.org.au/v4/data/network/WEM?metrics=power&interval=fy
This is likely because not all kinds are aggregated to all resolutions and because they have different time ranges allowed. 5 min for example is max 7 days.
| date_start: str | None = None, | ||
| date_end: str | None = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
end should be either now or target_datetime and start should be now or target - 3 days.
| derived_interval = _derive_interval_from_datetime(target_datetime) | ||
|
|
||
| # Build query parameters | ||
| params = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this should be a dict with a key value pair.
Issue
Closes #8415
Description
This PR aims to migrate the Open Electricity API used for Australian data to V4. Specifically for pricing.
Preview
Double check
poetry run test_parser "zone_key"pnpx prettier@2 --write .andpoetry run formatin the top level directory to format my changes.