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

Delete node count #1161

Merged
merged 11 commits into from
Mar 11, 2020
Merged

Delete node count #1161

merged 11 commits into from
Mar 11, 2020

Conversation

miovd
Copy link
Contributor

@miovd miovd commented Feb 17, 2020

Please check if the PR fulfills these requirements (please use '[x]' to check the checkboxes, or submit the PR and then click the checkboxes)

  • 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)

Does this PR already have an issue describing the problem ?
fix #781
fix #1118

What kind of change does this PR introduce?
Bug fix

Does this PR introduce a breaking change or deprecate an API? If yes, check the following:

  • The Breaking Change or Deprecated label has been added
  • The migration guide has been updated in the github wiki (What changes might users need to make in their application due to this PR?)

Other information:
In this proposition, I kept the same behaviour for NodeBreakerVoltageLevel.cleanTopology() than NodeBreakerVoltageLevel.clean() used to have with the same issues: hence, when the topology is cleaned and intermediate nodes are removed, they cannot be used again e.g. if a node/breaker voltage level only used nodes 1, 2 and 4 and is cleaned, attempting to use node 3 will throw a PowsyblException with the message Vertex 3 not found. Is this the behavior we want?

@miovd miovd added this to PR in progress in Evolutions IIDM via automation Feb 18, 2020
@miovd miovd changed the title [WIP] Delete node count Delete node count Feb 18, 2020
@mathbagu
Copy link
Contributor

@MioRtia I think it's strange to not be able to reuse a vertex. How can I replace an injection by another? If it's supported by the API (remove + add), it's OK for me to keep it like this

@miovd miovd changed the title Delete node count [WIP] Delete node count Feb 19, 2020
@miovd
Copy link
Contributor Author

miovd commented Feb 19, 2020

@MioRtia I think it's strange to not be able to reuse a vertex. How can I replace an injection by another? If it's supported by the API (remove + add), it's OK for me to keep it like this

With this proposition, when an injection (for example) is removed and another replaces it, there is no issue. There is an issue however, when cleanTopology is called between the two actions. I'm wondering how useful should this method be? As I said above, if we don't want to create "superfluous" vertices, it can be deleted (it will avoid most of issues I think).

@miovd miovd changed the base branch from master to evolution_xiidm/1.2 February 28, 2020 14:29
@miovd miovd changed the title [WIP] Delete node count Delete node count Feb 28, 2020
@miovd miovd changed the title Delete node count [WIP] Delete node count Mar 2, 2020
@miovd miovd changed the title [WIP] Delete node count Delete node count Mar 2, 2020
@miovd miovd changed the title Delete node count [WIP] Delete node count Mar 3, 2020
@miovd miovd changed the base branch from evolution_xiidm/1.2 to master March 4, 2020 08:20
@miovd miovd changed the title [WIP] Delete node count Delete node count Mar 4, 2020
@miovd miovd requested a review from mathbagu March 4, 2020 15:58
@miovd miovd added the Deprecated Methods have been deprecated label Mar 9, 2020
…oltage levels

Signed-off-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>

 correct code smells

Signed-off-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>

Update javadoc on VoltageLevel.cleanTopology()

Signed-off-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>

Add removeInternalConnection + Change mechanism (delete cleanTopology and auto-clean each time a connectable, a switch or an internal connection is removed)

Signed-off-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>
Signed-off-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>
Signed-off-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>
Signed-off-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>
Signed-off-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>
Signed-off-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>
Signed-off-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>
Signed-off-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>
Signed-off-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>
Signed-off-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>
@sonarcloud
Copy link

sonarcloud bot commented Mar 11, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 3 Code Smells

81.8% 81.8% Coverage
0.0% 0.0% Duplication

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Evolutions IIDM
  
Merged PR
3 participants