Fix new entrant renaming - #144
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests.
... and 4 files with indirect coverage changes 🚀 New features to boost your workflow:
|
39e2bf0 to
0a9569e
Compare
|
Hey Ellie - As discussed - perhaps could expand this out / make it slightly more readable? return new_entrants.groupby(
group_key_columns + ["geo_id"], dropna=False, as_index=False
).agg({"name": "first", **{col: "mean" for col in value_columns}})Something like: name_aggregation_rule = {"name": "first"}
value_column_aggregation_rules = {col: "mean" for col in value_columns}
return new_entrants.groupby(
group_key_columns + ["geo_id"], dropna=False, as_index=False
).agg(name_aggregation_rule | value_aggregation_rules)Or variation of .. Also maybe move :
From Otherwise looks good! |
| value_columns: list[str], | ||
| ) -> pd.DataFrame: | ||
| """Groups by ``group_key_columns`` + 'geo_id' and averages ``value_columns``.""" | ||
| # 'dropna=False' set to keep thermal generator rows (w/ NaN 'resource_type') |
There was a problem hiding this comment.
Was this comment incorrect?
There was a problem hiding this comment.
Oh nah just an over zealous 'cleaning' loss - might add back for context :)
|
|
||
| # Allowed 'extra' subregion (not in sub_regional_geography) present in the names of | ||
| # some new entrant gas plant; see Open-ISP/ISPyPSA#131 | ||
| _EXTRA_SUBREGION_IN_NAMES = {"WOO"} |
There was a problem hiding this comment.
Not sure that the tests cover this? i.e. that might all pass fine even without it.
There was a problem hiding this comment.
Actually if you make Dylan change move set(old_geo_ids) | _EXTRA_SUBREGION_IN_NAMES then I think the test you added would then cover this.
3 participants
Reworked the new entrants' renaming after regional granularity collapse/aggregation.
Previously: was just setting the new names to a standard '{geo_id} {technology}' formula; this lost some detail for units whose pre-collapse naming convention had different structures for representing technologies. Wasn't causing bugs or issues at this stage - but for mapping against constraints from plexos files or other AEMO-sourced name-keyed values, losing the names' structure had potential to be annoying AND was an easy fix. (+ keeps consistent with non-renamed units e.g. those in REZs).
Now: the actual 'old' geo_id string in a unit name gets re-keyed to the new geo_id (collapsed), keeping the rest of the name as-is. The first 'name' value for each collapsed-group is kept upon aggregation; this is the name that gets used to re-key. This implementation does assume that the naming convention for aggregated units (grouped on technology, geo_id and optionally resource_type) is the same for all units in each group, and that the 'new' geo_id is correctly assigned regardless of the old geo_id-like string prefix in each name. Based on current 7.5 workbook data this holds. Special case 'BOTN - Cethana' doesn't get aggregated with anything else so name gets passed through unchanged.
Example to illustrate the difference:
Built off #143