feat(pa): use json APIs for PA parser #6623
Conversation
github-actions
bot
added
parser
python
| net_flow_country | ||
| if sorted_exchange_key.endswith("PA") | ||
| else -net_flow_country | ||
| ) |
There was a problem hiding this comment.
issue(maybe): not an issue for this PR (also because I don't think that there is any exchange using this parser) but I don't think that this function is / was calculating the correct things (unless I am understanding things wrong 😅 )
Looking at https://sitr.cnd.com.pa/m/pub/int.html, the flows are between neighbouring countries, not all to PA. Also if we tried fetch_exchange() on any other combination than "CR->PA" it indeed fails saying that those are not valid exchanges.
Should I just remove this function? If not also happy to try fixing it in another PR
There was a problem hiding this comment.
Since this is unused it would be fine to remove it but if you want to fix it that would also be great 🙂
Totally up to you though IMO.
| and unit_generation >= 0 | ||
| ): # Ignore self-consumption | ||
| unit_fuel_type = MAP_THERMAL_GENERATION_UNIT_NAME_TO_FUEL_TYPE[unit_name] | ||
| production_mix.add_value(unit_fuel_type, unit_generation) |
There was a problem hiding this comment.
note: maybe the only downside of using directly the json API here is that now we cannot know dynamically which plants are "thermic" plants (because there is no way to know from the json which units are displayed on the webpage under the "Termicas" table). So if a new unknown fossil plant was to be built we wouldn't sum it up / get a warning. So far though we "know" of all existing plants 🙌🏽
There was a problem hiding this comment.
It's fine that the mapping is static but we should add a fallback so if a new plant is added and the type is "Thermica" and not included in the thermal mapping it gets added to unknown and logs a warning.
There was a problem hiding this comment.
Unfortunately the data is coming down like this https://sitr.cnd.com.pa/m/pub/data/gen.json , where "units" is just a long list of energy plant names and associated production - without any information about which plants are "Thermica", which are solar, which are hydroelectric, etc...
The grouping into production modes is introduced only at the frontend level https://sitr.cnd.com.pa/m/pub/gen.html (couldn't quite figure out if they had a dynamic way to render that or if essentially they are hard coding which plant goes into which table) so I fear that there is not really a way to be able to know which plants are thermic without either "knowing it" or scraping the html page...
Happy to revert if risking to miss out on new plants is a blocker!
There was a problem hiding this comment.
As long as the new plants are mapped to unknown so we get the total power produced it should be fine. Having less than the total can lead to the flowtracing algorithm not being able to find a valid solution if say the exports exceed the imports + production.
I doubt that will happen here since it's on a plant level but better be safe than sorry.
Description
This PR updates the
PAparser to use directly the backend json APIs rather than having to scrape the frontend HTML pages for data.This should hopefully make the code more readable / easier to test.
Double check
poetry run test_parser "zone_key"pnpx prettier@2 --write .andpoetry run formatin the top level directory to format my changes.