Intern cell explicit positionning #292

BenoitJeanson · 2021-10-22T13:23:08Z

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 ? If so, link to this issue using '#XXX' and skip the rest

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
feature

What is the current behavior? (You can also link to an open issue here)
Only ExternCells have an order.

What is the new behavior (if this is a feature change)?
Introduce ability to define an order, and explicit orientation to a vertical InternCell.
No Extension is available in iidm to support that. See TestInternCellExplicitPosition.java

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:

(if any of the questions/checkboxes don't apply, please delete them entirely)

Signed-off-by: BenoitJeanson <benoit.jeanson@rte-france.com>

Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>

...m-core/src/main/java/com/powsybl/sld/layout/positionfromextension/PositionFromExtension.java

Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>

single-line-diagram-core/src/main/java/com/powsybl/sld/layout/BlockPositionner.java

flo-dup · 2021-11-08T14:58:46Z

single-line-diagram-core/src/main/java/com/powsybl/sld/model/Node.java

@@ -58,6 +59,12 @@

    private String label;

+    private Optional<Integer> order = Optional.empty();


Optional should not be used as parameter of a class or a method, but rather only in return values.

Just some more arguments, as I didn't manage to convince you; intellij linter makes it a warning which says

Optional was designed to provide a limited mechanism for library method return types in which a clear way to represent "no result" was needed. Using a field with the java.util.Optional type is also problematic if the class needs to be Serializable, as java.util.Optional is not serializable.

See also the POJOs paragraph in http://dolszewski.com/java/java-8-optional-use-cases/

single-line-diagram-core/src/main/java/com/powsybl/sld/model/Node.java

single-line-diagram-core/src/test/java/com/powsybl/sld/raw/TestLanesWithUnileg.java

single-line-diagram-core/src/test/resources/InternalBranchesBusBreakerH.json

single-line-diagram-core/src/main/java/com/powsybl/sld/model/AbstractBusCell.java

Signed-off-by: BenoitJeanson <benoit.jeanson@rte-france.com>

Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>

flo-dup · 2021-11-19T17:14:45Z

...m-core/src/main/java/com/powsybl/sld/layout/positionfromextension/PositionFromExtension.java

+        graph.getNodes().stream().filter(node -> node.getDirection() != Direction.UNDEFINED).forEach(node -> {
+            BusCell cell = (BusCell) node.getCell();
+            cell.setDirection(node.getDirection());
+            cell.setOrder(node.getOrder().orElse(null));


Here we remove the order if node.getOrder().isEmpty(). I think that's dangerous, if one node of the cell has no order it might remove it. I'd rather write node.getOrder().ifPresent(cell::setOrder)

flo-dup · 2021-11-19T17:28:09Z

single-line-diagram-core/src/main/java/com/powsybl/sld/model/FeederNode.java

    @Override
    protected void writeJsonContent(JsonGenerator generator) throws IOException {
        super.writeJsonContent(generator);
        generator.writeStringField("feederType", feederType.name());
-        generator.writeNumberField("order", order);
+        generator.writeNumberField("order", getOrder().orElse(-1));


We should avoid writing -1 as it's the same as no information. I'll replace this line with getOrder().ifPresent(order -> generator.writeNumberField("order", order))

flo-dup · 2021-11-19T17:44:40Z

single-line-diagram-core/src/main/java/com/powsybl/sld/model/Node.java

@@ -58,6 +59,12 @@

    private String label;

+    private Optional<Integer> order = Optional.empty();


Just some more arguments, as I didn't manage to convince you; intellij linter makes it a warning which says

Optional was designed to provide a limited mechanism for library method return types in which a clear way to represent "no result" was needed. Using a field with the java.util.Optional type is also problematic if the class needs to be Serializable, as java.util.Optional is not serializable.

See also the POJOs paragraph in http://dolszewski.com/java/java-8-optional-use-cases/

flo-dup · 2021-11-19T17:47:13Z

single-line-diagram-core/src/main/java/com/powsybl/sld/model/AbstractBusCell.java

+    }
+
+    @Override
+    public void setOrder(Integer order) {


I don't think we need to use Integer instead of int, as we never remove the order. If so one day, it should be done more explicitly via a removeOrder. I'll add such a method just in case

flo-dup · 2021-11-19T17:47:45Z

single-line-diagram-core/src/main/java/com/powsybl/sld/model/BusCell.java

@@ -40,6 +40,12 @@ public Orientation toOrientation() {

    int newHPosition(int hPosition);

+    Optional<Integer> getOrder();
+
+    void setOrder(Integer order);


As said above, I'll replace Integer with int

Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>

sonarqubecloud · 2021-12-16T15:46:51Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

86.6% Coverage
0.0% Duplication

BenoitJeanson force-pushed the internCellExplicitPositionning branch from 77aac53 to ffff4f1 Compare October 22, 2021 13:24

BenoitJeanson added 3 commits October 22, 2021 18:09

intern cell explicit positionning

5f4362e

Signed-off-by: BenoitJeanson <benoit.jeanson@rte-france.com>

debug - clean up

664f2f8

Signed-off-by: BenoitJeanson <benoit.jeanson@rte-france.com>

sonar

e3d9c38

Signed-off-by: BenoitJeanson <benoit.jeanson@rte-france.com>

flo-dup force-pushed the internCellExplicitPositionning branch from 7b1dcbe to e3d9c38 Compare October 22, 2021 16:10

flo-dup and others added 2 commits November 8, 2021 12:43

Merge branch 'main' into internCellExplicitPositionning

3e76dfc

Revert formatting changes

e5953e4

Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>

flo-dup reviewed Nov 8, 2021

View reviewed changes

...m-core/src/main/java/com/powsybl/sld/layout/positionfromextension/PositionFromExtension.java Outdated Show resolved Hide resolved

Remove TODO following creation of corresponding issue #299

0bf0b94

Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>

flo-dup requested changes Nov 8, 2021

View reviewed changes

flo-dup force-pushed the internCellExplicitPositionning branch from 0bf0b94 to e3d9c38 Compare November 10, 2021 09:27

BenoitJeanson added 6 commits November 10, 2021 11:27

PR review suggested changes - except Cell order

ebcb291

Signed-off-by: BenoitJeanson <benoit.jeanson@rte-france.com>

PR suggestion on order as Integer + Optional management

284fff5

Signed-off-by: BenoitJeanson <benoit.jeanson@rte-france.com>

debug obvious unit tests

87360a8

Signed-off-by: BenoitJeanson <benoit.jeanson@rte-france.com>

Debug + test

5db6d8c

Signed-off-by: BenoitJeanson <benoit.jeanson@rte-france.com>

checkstyle

17395b9

Signed-off-by: BenoitJeanson <benoit.jeanson@rte-france.com>

sonar

3041ec1

Signed-off-by: BenoitJeanson <benoit.jeanson@rte-france.com>

flo-dup force-pushed the internCellExplicitPositionning branch 2 times, most recently from bcaccbe to 19c2a90 Compare November 18, 2021 13:47

flo-dup added 2 commits November 18, 2021 14:49

Revert formatting changes

9f553d8

Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>

Update gitignore

494d229

Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>

flo-dup force-pushed the internCellExplicitPositionning branch from 19c2a90 to 494d229 Compare November 18, 2021 13:50

flo-dup and others added 8 commits November 18, 2021 14:50

Merge branch 'main' into internCellExplicitPositionning

8cdadc6

Do not remove the order of the BusCell if the node has no order

1c2a1ed

Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>

Do not write order in json if missing

10a0b9e

Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>

Replace average loop by lambda

119b7f2

Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>

Replace Optional parameter with Integer parameter

d701905

Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>

Feeder order is optional to avoid problems with value -1

6fa0032

Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>

Add removeOrder

4d78fe0

Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>

average moved to PositionFromExtension

43af33f

Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>

flo-dup added 3 commits December 16, 2021 16:23

Only add order info of the one leg in the vl for 3wt

82b0308

Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>

Update unit tests

4fdcbe7

Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>

Merge branch 'main' into internCellExplicitPositionning

e98fb2a

Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>

flo-dup reviewed Dec 16, 2021

View reviewed changes

RawGraphBuilder: add extension even if direction is null

fa6d440

Signed-off-by: Florian Dupuy <florian.dupuy@rte-france.com>

flo-dup merged commit 0e9b37a into main Dec 16, 2021

flo-dup deleted the internCellExplicitPositionning branch December 16, 2021 16:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Intern cell explicit positionning #292

Intern cell explicit positionning #292

BenoitJeanson commented Oct 22, 2021 •

edited by flo-dup

Loading

flo-dup Nov 8, 2021

flo-dup Nov 19, 2021

flo-dup Nov 19, 2021

flo-dup Nov 19, 2021

flo-dup Nov 19, 2021

flo-dup Nov 19, 2021

flo-dup Nov 19, 2021

sonarqubecloud bot commented Dec 16, 2021

		@@ -58,6 +59,12 @@

		private String label;

		private Optional<Integer> order = Optional.empty();

Intern cell explicit positionning #292

Intern cell explicit positionning #292

Conversation

BenoitJeanson commented Oct 22, 2021 • edited by flo-dup Loading

flo-dup Nov 8, 2021

Choose a reason for hiding this comment

flo-dup Nov 19, 2021

Choose a reason for hiding this comment

flo-dup Nov 19, 2021

Choose a reason for hiding this comment

flo-dup Nov 19, 2021

Choose a reason for hiding this comment

flo-dup Nov 19, 2021

Choose a reason for hiding this comment

flo-dup Nov 19, 2021

Choose a reason for hiding this comment

flo-dup Nov 19, 2021

Choose a reason for hiding this comment

sonarqubecloud bot commented Dec 16, 2021

BenoitJeanson commented Oct 22, 2021 •

edited by flo-dup

Loading