-
Notifications
You must be signed in to change notification settings - Fork 901
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 merge Exchanges #5408
Add merge Exchanges #5408
Conversation
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.
Bunch of small comments in regards to types plus some general questions and suggestions.
Will be nice to get this added!
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.
I added a suggestion to change the docstring but beyond that this looks good to me.
I also took the liberty to format the files and fix a small spelling error. I hope you don't mind.
As discussed with @mathilde-daugy offline we decided to rename it to AggregatableEventList. |
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.
Looks better :)
Issue
As mentioned on #5350, we also need a way of merging exchanges inside the new list datastructures as this is needed in the ENTSO-E parser at least. As it is looking at imports first and then merging with exports.
Description
Double check
poetry run test_parser "zone_key"
pnpx prettier --write .
andpoetry run format
to format my changes.