Migrate to new report API #139
Conversation
Signed-off-by: Luma <zamarrenolm@aia.es>
Signed-off-by: Luma <zamarrenolm@aia.es>
Signed-off-by: Luma <zamarrenolm@aia.es>
Signed-off-by: Luma <zamarrenolm@aia.es>
Signed-off-by: Luma <zamarrenolm@aia.es>
Signed-off-by: Luma <zamarrenolm@aia.es>
Signed-off-by: Luma <zamarrenolm@aia.es>
Signed-off-by: Luma <zamarrenolm@aia.es>
|
|
||
| BalanceComputationRunningContext context = new BalanceComputationRunningContext(areas, network, parameters, reporter); | ||
| BalanceComputationRunningContext context = new BalanceComputationRunningContext(areas, network, parameters, reportNode); |
There was a problem hiding this comment.
Just wondering why we give two reportNodes to the context even though it's useless 🤨 Not the purpose of this PR anyway!
There was a problem hiding this comment.
Agree, it is not clear that the two report nodes are needed. I just kept the same behaviours, just upgrading to the new report API.
There was a problem hiding this comment.
@zamarrenolm you may clarify via comments that (please adjust with new reporting api terminology, unsure of correct new terms ...):
- reportNode is the root reporter / provided when calling run
- iterationReportNode is subreporter for the current balancing iteration
There was a problem hiding this comment.
Updated with comments. Also used the method to obtain the root report node from the context, to make explicit that the balance adjustment different steps have it available through the context as eventually the complexity of the current calculation could be split in many lower level methods.
There was a problem hiding this comment.
Thanks Luma for clarifying with comments. Nonetheless I think it would be clearer to remove the two ReportNodes from the context, as hiding them in this object does not help understanding the reports split... but again it's not the purpose of current PR!
There was a problem hiding this comment.
@flo-dup I don't understand previous comments '... it's useless ...', 'it would be clearer to remove the two ReportNodes', '... hiding ...'
The two report nodes are different, they are both useful, and should not be removed.
They are not hidden in the context, it is the opposite, they are made available via the context, so they can actually be used.
...y-check-ucte/src/test/java/com/powsybl/glsk/ucte/quality_check/GlskQualityProcessorTest.java
Outdated
Show resolved
Hide resolved
…shot Signed-off-by: Luma <zamarrenolm@aia.es>
Signed-off-by: Luma <zamarrenolm@aia.es>
Signed-off-by: Luma <zamarrenolm@aia.es>
|
Included in #141 |
Please check if the PR fulfills these requirements
Does this PR already have an issue describing the problem?
What kind of change does this PR introduce?
What is the current behavior?
What is the new behavior (if this is a feature change)?
Does this PR introduce a breaking change or deprecate an API?
If yes, please check if the following requirements are fulfilled
What changes might users need to make in their application due to this PR? (migration steps)
Other information: