Skip to content
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 aux flows to topology #129

Merged
merged 10 commits into from
Oct 22, 2023
Merged

Add aux flows to topology #129

merged 10 commits into from
Oct 22, 2023

Conversation

olejandro
Copy link
Member

This PR adds aux commodity flows to topology. It also expands the list of Veda aliases for TIMES attributes.

@olejandro
Copy link
Member Author

@siddharth-krishna this one has regression with respect to addtional rows, so still needs some work.
Btw, was wondering whether this pattern where we are looping through the dictionary searching for a key is efficient? Please feel free to modify...

times_reader/transforms.py Outdated Show resolved Hide resolved
@Antti-L
Copy link

Antti-L commented Oct 12, 2023

@olejandro

Expand aliases_by_attr

Just a small question/comment about this alias matter: I wonder whether you take into account that there are both strict aliases and variants of parameters now included in the lists. For example, CF is not strictly an alias for NCAP_AF, because the default LimType for CF (alias UTILIZATION) is FX while it is UP for NCAP_AF (alias AF, AVAILABILITY). A few other examples are EFF, which is not a strict alias for ACT_EFF, but a variant which does not support specifying the CG in ACT_EFF at all, and COST, which is not a strict alias for IRE_PRICE, but a variant which does not support specifying the commodity or IE, and ENV_ACT, which is not strictly an alias for FLO_EMIS, but a variant which does not support specifying the source CG.

@olejandro
Copy link
Member Author

@Antti-L good point! Yes, we already include a dictionary that specifies (some of) those defaults (incl. whether it should be input or output commodity like for share-o vs share-i). The idea is to move this info to a separate file, so that it is easy to get an overview and modify it. I'll make sure to tag you in that PR, so you can comment on it.

@olejandro
Copy link
Member Author

@siddharth-krishna there is one additional record on Ireland which I believe is okay to keep. :-)

@siddharth-krishna
Copy link
Collaborator

Sounds good. Let me think what's the best way to merge this PR with a failing check..

@olejandro
Copy link
Member Author

@siddharth-krishna I was thinking to temporarily change the repository settings to be able to do this (i.e. not to enforce restrictions for admins).

@siddharth-krishna
Copy link
Collaborator

Sounds good!

@olejandro olejandro merged commit 0f7793a into main Oct 22, 2023
1 check failed
@olejandro olejandro deleted the olex/improve-topology branch October 22, 2023 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants