-
Notifications
You must be signed in to change notification settings - Fork 961
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
Add data parser for Iceland #338
Conversation
Whoops this should only be my one latest commit, I'm not sure why it's listing several? The rest are the same as mainline anyway. |
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.
Can you add geothermal as a production source in the code? The carbon intensity of this technology should be set at 24gCO2eq/kWh, as per IPCC 2014 median value. Thanks!
feeder/feeder.py
Outdated
@@ -300,7 +302,7 @@ def validate_production(obj, country_code): | |||
raise Exception("Data from %s can't be in the future" % country_code) | |||
if obj.get('production', {}).get('unknown', None) is None and \ | |||
obj.get('production', {}).get('coal', None) is None and \ | |||
country_code not in ['CH', 'NO']: | |||
country_code not in ['CH', 'NO', 'IS']: |
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.
Please remove from here. In principle, because you have "unknown" not None, this should not be a problem.
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.
Now that I've added support for geothermal this is required again.
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.
The purpose of this line is to make exceptions for specific countries that we want to show on the map even though they have no coal data.
However, having no coal data is different from having coal set to 0. For IS, if we can confirm that they do not produce any coal, we should set the coal
key to 0 in the parser. We should do the same for all production types that we know IS doesn't have.
feeder/parsers/IS.py
Outdated
|
||
# Calculate production values for each power station | ||
for key, value in STATIONS.items(): | ||
items = [item for item in json_obj if item["substation"] == key and item["MW"] >= 0] |
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.
what happens to all stations that are not matched? They are simply ignored?
We should add them to unknown
maybe?
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.
They are ignored, yeah. I'm not sure it's a good idea to funnel them all into unknown by default because I don't think everything we get through that API is actually power generation.
For example, there is a substation BRENNIME
which Google translates/expands to Brennimelur
which seems to be a capacitor bank: http://www.rst.is/en/verkefni/2001/BRE-30.html not a generator/power station.
The data for that example has some pretty big MW values, so it'd skew our results quite heavily. Without better knowledge of Icelandic I was thinking it might be best for now to just stick with the stations that @birkir made specific note of? What do you think?
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.
ok I agree! Maybe add a small comment explaining that in the code.
feeder/feeder.py
Outdated
@@ -300,7 +302,7 @@ def validate_production(obj, country_code): | |||
raise Exception("Data from %s can't be in the future" % country_code) | |||
if obj.get('production', {}).get('unknown', None) is None and \ | |||
obj.get('production', {}).get('coal', None) is None and \ | |||
country_code not in ['CH', 'NO']: | |||
country_code not in ['CH', 'NO', 'IS']: |
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.
The purpose of this line is to make exceptions for specific countries that we want to show on the map even though they have no coal data.
However, having no coal data is different from having coal set to 0. For IS, if we can confirm that they do not produce any coal, we should set the coal
key to 0 in the parser. We should do the same for all production types that we know IS doesn't have.
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.
Great job! Can you please update capacities.json and Readme with the following information:
Iceland Installed Power Generation Capacity (Jan 2015)
- Hydro: 1986MW
- Geothermal: 665MW
- Wind: 3MW
- Oil: 117MW
All the rest is 0MW
Source: Statistics Iceland http://px.hagstofa.is/pxen/pxweb/en/Atvinnuvegir/Atvinnuvegir__orkumal/IDN02101.px
LGTM after the changes requested by @brunolajoie are done! |
Good job! |
There is a mismatch between hydro power plant capacities and productions from the API. For instance, the BURF_F hydro power plant has a capacity of 6*45=270MW http://www.landsvirkjun.com/Company/PowerStations/BurfellPowerStation But on http://amper.landsnet.is/MapData/api/measurements we have 6 values for each turbine, summing up to 394MW! Given that the capacity figures have been double checked in various websites, i'm rather confident that the issue comes from the amper.landsnet.is API. |
One downside of adding the geothermal energy to 'unknown' is it makes Iceland look more polluting when it'd actually be one of the greenest countries on the map!
Also, I don't think this data is totally complete. For example there's some missing wind energy that we don't have data for (although it would be a tiny fraction overall).
For future reference I've left some comments about power stations I don't think we're getting data for and which stations are actually geothermal if we do add that one day. :)
Fixes #140