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 getI() method to DanglingLine's boundary #3168

Merged
merged 18 commits into from
Nov 28, 2024
Merged

Conversation

caioluke
Copy link
Member

@caioluke caioluke commented Oct 4, 2024

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce?

Feature

What is the current behavior?

Boundary nodes do not have a getI() method, whereas terminals do. Therefore, for paired dangling lines, we can not get the current from both sides of the half line easily.

What is the new behavior (if this is a feature change)?
Add the getI() method for boundary nodes.

Does this PR introduce a breaking change or deprecate an API?

  • Yes
  • No

If yes, please check if the following requirements are fulfilled

  • The Breaking Change or Deprecated label has been added
  • The migration steps are described in the following section

What changes might users need to make in their application due to this PR? (migration steps)

Custom IIDM implementations that are implementing the Boundary interface (which is not necessarily the case) should add the following method to their implementation:

  • double getI();

Signed-off-by: Caio Luke <caioluke97@gmail.com>
@caioluke caioluke requested a review from jeandemanged October 4, 2024 14:32
@caioluke caioluke self-assigned this Oct 4, 2024
@jeandemanged jeandemanged changed the title Add getI method to boundary node Add getI() method to DanglingLine's boundary Oct 4, 2024
@jeandemanged jeandemanged changed the title Add getI() method to DanglingLine's boundary Add getI() method to DanglingLine's boundary Oct 4, 2024
caioluke and others added 8 commits October 4, 2024 17:19
Signed-off-by: Caio Luke <caioluke97@gmail.com>
Signed-off-by: Caio Luke <caioluke97@gmail.com>
Signed-off-by: Caio Luke <caioluke97@gmail.com>
Signed-off-by: Caio Luke <caioluke97@gmail.com>
Signed-off-by: Caio Luke <caioluke97@gmail.com>
Signed-off-by: Caio Luke <caioluke97@gmail.com>
Signed-off-by: Caio Luke <caioluke97@gmail.com>
@jeandemanged
Copy link
Member

Hi @olperr1 ,
One question, I am not so sure this should be flagged "Breaking Change". Unless there are iIDM implementations around overriding the default "utils" calculations that are built-in the iIDM API. But I don't think implementations should do that ...
Your opinion ?

@olperr1
Copy link
Member

olperr1 commented Nov 14, 2024

Hi @olperr1 , One question, I am not so sure this should be flagged "Breaking Change". Unless there are iIDM implementations around overriding the default "utils" calculations that are built-in the iIDM API. But I don't think implementations should do that ... Your opinion ?

Hi @jeandemanged,
You are right, I was misled by the interface's package. It is not used by network elements, but only by utils. I will remove the label.

@olperr1 olperr1 removed the Breaking Change API is broken label Nov 14, 2024
@jeandemanged jeandemanged requested a review from olperr1 November 27, 2024 17:34
@olperr1
Copy link
Member

olperr1 commented Nov 28, 2024

I double-checked the Boundary interface usage and I found that powsybl-network-store has its own implementation (which is very similar to the powsybl-core one, but its an other problem). So I will report this change as a breaking change.

@olperr1 olperr1 added Breaking Change API is broken and removed PR: waiting-for-review labels Nov 28, 2024
@olperr1 olperr1 merged commit d876693 into main Nov 28, 2024
7 checks passed
@olperr1 olperr1 deleted the add_get_I_to_boundary branch November 28, 2024 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants