New trace mapping approach (renewable energy resources only)#52
Conversation
…n string (2024 by default)
(This will be changed / refactored later)
(This will be changed / refactored later)
- Yaml structure for wind was previously different to solar and needed some unpacking before use - Also updated a default arg in example docs
(explains rationale for change, options considered etc)
Minor typo's / fixes to issue linking
Codecov Report✅ All modified and coverable lines are covered by tests.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
EllieKallmier
left a comment
EllieKallmier
left a comment
There was a problem hiding this comment.
Looks good to me :) I think this schema is pretty easy to follow from a readability perspective, and looks like it'll help simplify some things which is nice! Just had the one note about T5 not really existing (lol) otherwise happy as. Also found the ADR write-up pretty helpful to really understand the impacts/changes this schema introduction will have elsewhere in the package and the rationale behind it all.
| T4: TAS # North West Tasmania Coast | ||
| T5: TAS # North East Tasmania Coast |
There was a problem hiding this comment.
I don't think T5 actually exists in the 2024 ISP - and T4 was named "North Tasmania Coast" in the end. Confusingly though T5 is present in the new entrant generator summary tables in v6.0 of the IASR workbook (and these T4/T5 names are used), it's not used consistently through the workbook and other tables, and is present but crossed out in some spots (e.g. Capacity Factors sheet). Basically I think this should just be T4: TAS # North Tasmania Coast :)
There was a problem hiding this comment.
Ha - how funny - I had meant to mention this.. I had the exact same issue / question about this - but jumped the other way (I saw it referred to in some places but not others). Also the trace names have T5:
T5_WFL_North_East_Tasmania_Coast_RefYear2011.csv
T5_WFX_North_East_Tasmania_Coast_RefYear2011.csv
Seems like it should be T4 afterall - good work on spotting!
Incidentally possibly shows a good example of value of mapping of regex - this would be annoying edge case to deal with in regex, but easy to change the mapped zone from T5 to T4...
nick-gorman
left a comment
nick-gorman
left a comment
There was a problem hiding this comment.
Looks good to me Dylan! I like the removal of the regex, and all the mappings are clear and easy to follow.
(T5 didn't really exist in the final 2024 ISP, but did in the trace names in some older IASR workbooks. See PR #52)
3 participants
Restructures
src/isp_trace_parser/mappings/to a per-ISP-version layout and replaces the four legacy YAMLs with unifiedresources.yaml+topography.yaml. Seedocs/adr/001-trace-name-mapping.mdfor rationale and schema.Files
mappings/2024/resources.yaml,mappings/2024/topography.yaml,docs/adr/001-trace-name-mapping.mdsolar_project_mapping.yaml,solar_zone_mapping.yaml,wind_project_mapping.yaml,wind_zone_mapping.yamlCode
mappings/__init__.py: loader now takes an ISP-version arg (defaults to"2024").solar_traces.py,wind_traces.py: updated to load from the new unified structure.wind_traces.py: removed an unused helper.Notes
There is pretty minimal changes to the actual code (basically just small inline functions to load the new data), and removed a now unused helper function. Don't pay too much attention to these functions - they are basically just place holders, that will be modified / refactored down the track. Is the code base doesn't really take advantage of the new structure in anyway yet - that will come (mainly an effort in not try to boil the ocean, and make sure don't inadvertently break anything).
This PR is basically just for the resources traces and 2024 data - demand / and 2026 additions will also come later (for similar reasons) - Main changes are basically just updates / consolidation of the wind/solar yaml files (and large ADR to explain rationale / why)