Modify nad and sld api for better consistency #475

So-Fras · 2023-01-13T16:08:09Z

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
#469

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

sonarcloud · 2023-01-23T09:23:37Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
17 Code Smells

51.4% Coverage
0.0% Duplication

…d signature to include filter Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>

Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>

…etrization Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>

Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>

…arameters Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>

Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>

Signed-off-by: Thomas ADAM <tadam@silicom.fr>

flo-dup

Good job! but I think there's still some work/rethinking before it is ready

flo-dup · 2023-03-28T15:44:15Z

network-area-diagram/src/main/java/com/powsybl/nad/NetworkAreaDiagramConfiguration.java

@@ -0,0 +1,52 @@
+package com.powsybl.nad;


Copyright and authorship are missing for the new files of this PR

flo-dup · 2023-03-28T15:45:45Z

network-area-diagram/src/main/java/com/powsybl/nad/NetworkAreaDiagramConfiguration.java

+import com.powsybl.nad.svg.StyleProvider;
+import com.powsybl.nad.svg.SvgParameters;
+
+public class NetworkAreaDiagramConfiguration {


We could simplify this with DiagramConfig, or just Config?

flo-dup · 2023-03-28T15:47:57Z

network-area-diagram/src/main/java/com/powsybl/nad/NetworkAreaDiagramConfigurationBuilder.java

+ LayoutParameters layoutParameters = new LayoutParameters();
+ StyleProvider styleProvider;
+ LabelProvider labelProvider;
+ BiFunction<Network, SvgParameters, LabelProvider> labelProviderCreator = DefaultLabelProvider::new;


Suggested change

BiFunction<Network, SvgParameters, LabelProvider> labelProviderCreator = DefaultLabelProvider::new;

LabelProviderFactory labelProviderFactory = DefaultLabelProvider::new;

flo-dup · 2023-03-28T15:48:43Z

network-area-diagram/src/main/java/com/powsybl/nad/NetworkAreaDiagramConfigurationBuilder.java

+ SvgParameters svgParameters = new SvgParameters();
+ LayoutParameters layoutParameters = new LayoutParameters();
+ StyleProvider styleProvider;
+ LabelProvider labelProvider;


you can remove this parameter, we shouldn't create it before build call.

Or in fact it should maybe be build only when calling config.getLabelProvider(). Indeed, one would expect one config to be network-agnostic, because you expect when speaking of config to reuse the same config for different networks. That implies that both StyleProvider and LabelProvider are created only when calling config.getLabelProvider(network). And then similarly for the idProvider.

flo-dup · 2023-03-28T17:53:07Z

network-area-diagram/src/main/java/com/powsybl/nad/NetworkAreaDiagramConfigurationBuilder.java

+ private static <R extends LabelProvider> R factory(Network network, SvgParameters svgParameters, BiFunction<Network, SvgParameters, R> function) {
+ return function.apply(network, svgParameters);
+ }


Suggested change

private static <R extends LabelProvider> R factory(Network network, SvgParameters svgParameters, BiFunction<Network, SvgParameters, R> function) {

return function.apply(network, svgParameters);

}

@FunctionalInterface

public interface LabelProviderFactory {

LabelProvider create(Network network, SvgParameters svgParameters);

}

flo-dup · 2023-03-28T18:08:36Z

network-area-diagram/src/test/java/com/powsybl/nad/NetworkAreaDiagramTest.java

+ .withSvgParameters(getSvgParameters())
+ .withStyleProvider(getStyleProvider(network))
+ .build();


indenting isn't right

flo-dup · 2023-03-28T19:40:18Z

...single-line-diagram-core/src/main/java/com/powsybl/sld/svg/AbstractDiagramLabelProvider.java


- protected AbstractDiagramLabelProvider(ComponentLibrary componentLibrary, LayoutParameters layoutParameters) {
+ protected AbstractDiagramLabelProvider(ComponentLibrary componentLibrary, LayoutParameters layoutParameters, SvgParameters svgParameters) {


we should be able to get rid of LayoutParameters here...

flo-dup · 2023-03-28T19:41:24Z

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

 return this;
 }

- public boolean isRemoveFictitiousSwitchNodes() {
- return removeFictitiousSwitchNodes;
+ public double getCgmesScaleFactor() {


Those cgmes parameters are unexpected here... The smart selector does not allow to pass any parameters, so LayoutParameters were used for cgmes-only parameters.
But ok to keep them in this PR while waiting for another PR to clean that. We could probably delete cmgesUseName, but cgmesScaleFactor and cgmesDiagramName are needed...

I agree, it does not feel like the right place to put those cgmes parameters. It was indeed a quick and dirty way to deal with them.
I created an issue to track that.

flo-dup · 2023-03-28T19:42:46Z

.../single-line-diagram-core/src/main/java/com/powsybl/sld/svg/DefaultDiagramLabelProvider.java

@@ -34,8 +34,8 @@ public class DefaultDiagramLabelProvider extends AbstractDiagramLabelProvider {

 protected final Network network;

- public DefaultDiagramLabelProvider(Network net, ComponentLibrary componentLibrary, LayoutParameters layoutParameters) {


As we try to have more consistency between diagrams, we could also take the opportunity to rename this into DefaultLabelProvider

flo-dup · 2023-03-28T20:26:21Z

...line-diagram-core/src/test/java/com/powsybl/sld/iidm/TestComplexParallelLegsInternalPst.java

+ SingleLineDiagramConfiguration singleLineDiagramConfiguration = new SingleLineDiagramConfigurationBuilder(network)
+ .withLayoutParameters(layoutParameters)
+ .withComponentLibrary(getResourcesComponentLibrary())
+ .withSvgParameters(svgParameters)
+ .build();


To avoid repeating this code block again and again, why don't you factorize it in toSvg? You could do that similarly to what was done before

Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>

…nting Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>

sonarcloud · 2023-03-29T09:57:37Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
4 Code Smells

93.9% Coverage
1.9% Duplication

…ers to SvgParameters Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>

Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>

So-Fras · 2023-09-27T09:16:48Z

See new PR #522

So-Fras added Breaking change NAD SLD labels Jan 13, 2023

So-Fras changed the title ~~[WIP] Modify nad ans sld api for better consistency~~ [WIP] Modify nad and sld api for better consistency Feb 3, 2023

So-Fras and others added 14 commits March 28, 2023 15:27

delete public network area diagram constructors and change draw metho…

9796a36

…d signature to include filter Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>

include prefixId data in new SvgParameters class for single line diagram

a28a309

Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>

fix on comments

8cc6cd6

Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>

reduce the number of draw function possible signatures

2dca5c3

Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>

add new class to deal with network area diagram configuration

9589a8b

Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>

add new class to deal with single line diagram configuration

f50503f

Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>

renaming adder --> builder

97a108f

Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>

fix problems after merging with main

aad6361

Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>

update test functions to use SingleLineDiagramConfiguration for param…

7f09aad

…etrization Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>

update cgmes classes and tests with renamed layout parameters

c78fcdd

Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>

update sld core after moving parameters from LayoutParameters to SvgP…

f3937ee

…arameters Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>

update test references

d01e87f

Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>

add test for svg parameters in sld

f4d3037

Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>

Fix testAnimation compilation issue

030f16d

Signed-off-by: Thomas ADAM <tadam@silicom.fr>

tadam50 force-pushed the modify_nad_api_for_better_consistency_with_sld branch from 4bb8f3d to 030f16d Compare March 28, 2023 13:42

So-Fras marked this pull request as ready for review March 28, 2023 14:43

So-Fras changed the title ~~[WIP] Modify nad and sld api for better consistency~~ Modify nad and sld api for better consistency Mar 28, 2023

So-Fras requested a review from flo-dup March 28, 2023 14:44

flo-dup requested changes Mar 28, 2023

View reviewed changes

So-Fras added 2 commits March 29, 2023 11:07

following review: add copyright and authorship when missing

f840a1f

Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>

renaming some classes with shorter / more consistent names + fix inde…

cdc6f6f

…nting Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>

fix in comment following some parameters migration from LayoutParamet…

6ea6e43

…ers to SvgParameters Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>

make ConfigBuilder network agnostic

7709829

Signed-off-by: Sophie Frasnedo <sophie.frasnedo@rte-france.com>

So-Fras mentioned this pull request Jun 5, 2023

Modify diagram apis #522

Merged

5 tasks

flo-dup force-pushed the main branch from 1a28286 to 0248753 Compare June 7, 2023 15:43

So-Fras closed this Sep 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modify nad and sld api for better consistency #475

Modify nad and sld api for better consistency #475

So-Fras commented Jan 13, 2023

sonarcloud bot commented Jan 23, 2023

flo-dup left a comment

flo-dup Mar 28, 2023

flo-dup Mar 28, 2023

flo-dup Mar 28, 2023

flo-dup Mar 28, 2023

flo-dup Mar 28, 2023

flo-dup Mar 28, 2023

flo-dup Mar 28, 2023

flo-dup Mar 28, 2023

So-Fras Mar 29, 2023 •

edited

Loading

flo-dup Mar 28, 2023

flo-dup Mar 28, 2023

sonarcloud bot commented Mar 29, 2023

So-Fras commented Sep 27, 2023

	BiFunction<Network, SvgParameters, LabelProvider> labelProviderCreator = DefaultLabelProvider::new;
	LabelProviderFactory labelProviderFactory = DefaultLabelProvider::new;

- private static <R extends LabelProvider> R factory(Network network, SvgParameters svgParameters, BiFunction<Network, SvgParameters, R> function) {
- return function.apply(network, svgParameters);
- }
+ @FunctionalInterface
+ public interface LabelProviderFactory {
+ LabelProvider create(Network network, SvgParameters svgParameters);
+ }


		protected AbstractDiagramLabelProvider(ComponentLibrary componentLibrary, LayoutParameters layoutParameters) {
		protected AbstractDiagramLabelProvider(ComponentLibrary componentLibrary, LayoutParameters layoutParameters, SvgParameters svgParameters) {

		@@ -34,8 +34,8 @@ public class DefaultDiagramLabelProvider extends AbstractDiagramLabelProvider {

		protected final Network network;

		public DefaultDiagramLabelProvider(Network net, ComponentLibrary componentLibrary, LayoutParameters layoutParameters) {

Modify nad and sld api for better consistency #475

Modify nad and sld api for better consistency #475

Conversation

So-Fras commented Jan 13, 2023

sonarcloud bot commented Jan 23, 2023

flo-dup left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

So-Fras Mar 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Mar 29, 2023

So-Fras commented Sep 27, 2023

So-Fras Mar 29, 2023 •

edited

Loading