create generic_branch #729
Fixes issue: #729 [FEATURE] Provide a generic_branch
Changes proposed in this PR include:
- added Component generic_branch as diskussed in #729
- added testcase generic_branch (self test of the new component)
- added testcase compare_generic_branch (compare a generic_branch with identical transformer)
Hi @sudo-ac,
Thanks for submitting the PR. Before I go in-depth of the content, I would like to give compliment for passing almost all the CI checks for the first try. This rarely happens, even for the active PGM developer.
Again: thank you for creating the PR. It looks quite good at first sight. I've already started going over this PR, but something else came up. Will continue with the review after that's solved.
One test failed, but we have no control over it:
************* Module power_grid_model.validation.validation /opt/hostedtoolcache/Python/3.10.15/x64/lib/python3.10/site-packages/power_grid_model/validation/validation.py:1:0: C0302: Too many lines in module (1001/1000) (too-many-lines)
One test failed, but we have no control over it:
************* Module power_grid_model.validation.validation /opt/hostedtoolcache/Python/3.10.15/x64/lib/python3.10/site-packages/power_grid_model/validation/validation.py:1:0: C0302: Too many lines in module (1001/1000) (too-many-lines)
Hi @sudo-ac , i'll see if i can come up with a good suggestion on this. The length of the module has always bothered me, but we've never had the time to refactor it. Unfortunately, you're the first one to experience a failing CI because of it. I'll dig into this on Monday.
@mgovers My proposal is to add a linter exception for this file? To refactor the whole validation function is out of the scope.
Maybe we need to define all the rules in json files. The validation function load the json file and perform certain check.
@sudo-ac I have added a silence line in the beginning of the file
@TonyXiang8787,
but I added the requested documentation. What was wrong?
@TonyXiang8787,
but I added the requested documentation. What was wrong?
Yes, indeed. Sorry I did not pay attention. I just put one more comment.
Hi @TonyXiang8787,
I added a test case in test_generic_branch, but I will review and update the missing validation on Monday.
Hi @TonyXiang8787,
Thank you very much for reviewing this PR. Have a nice weekend!
@mgovers My proposal is to add a linter exception for this file? To refactor the whole validation function is out of the scope.
Thank you @TonyXiang8787 for picking that up. I indeed agree that silencing for this PR is the way to go for now. Let's revisit the module after this PR is merged and try to come up with a sustainable solution.
Maybe we need to define all the rules in
jsonfiles. The validation function load thejsonfile and perform certain check.
I believe the entire validation functionality requires some rework. JSON files sound good, but more complex logic will be difficult to implement that way. Take for example the logic surrounding the default value of the tap position and the one concerning the control side of the transformer tap regulator