powsybl / powsybl-core

A framework to build power system oriented software
https://www.powsybl.org
Mozilla Public License 2.0
126 stars 41 forks source link

Keep reports and child reporters order #2190

Closed geofjamg closed 8 months ago

geofjamg commented 2 years ago

(if a question doesn't apply, you can delete it)

laurgarn commented 2 years ago

The currently defined interface already allows to interleave instances of what has been worded LogEvent (leaf) and LogContext (non leaf). But, as you point with this user story, the implementation does not keep this interleaving, as each kind is kept in its own sequence. So I'd say we have two options. 1) break nowaday interface and have Three kind of nodes : leaf, leaf container, non-leaf container. Hard to use and breaks existing calls. 2) have leaf and non-leaf insertable in a same sequence. In modern java, it means having both implement a common interface (LogItem in Convergence).

I think we should go the second option, but we'd be left with same problem I faced in Convergence. Because we use a tree (to navigate more easily through the hierarchy of contexts) and a table (to show all leaves under a tree node/context), but the tree helps less for leaves with non-leaf siblings. Especially, if a leaf sums up its closer context after "earlier" sibling that transitively contain a lot of leaves, user could : 1) see a red context with only green sub-contexts, wondering how 2) have to select the parent context and then scroll down to the end of the table to see the (red) sum up at the bottom.

In Convergence, I came up with a functionaly satisfying but aesthetic-al questionable soluton. In the front end, I create an "artificial context" for any (may be run of) leaf that has at least a non-leaf sibling, so it can be selected in the tree. The aesthetic-al annoyance is that the only "text" I could come up with for such artificial contexts was numbering them in their parent context. To be noticed that Youssef Akel asked last year to extend this solution to some other place I don't remember, though.

Not sure, how long it will take you to understand my writing. I will have to illustrate this with some screenshots, I guess

flo-dup commented 2 years ago

Thanks a lot @laurgarn for your feedback.

I'm not sure I understand exactly all of it though! I think there isn't the same problem in the new design of the Reporter, but I might have misunderstood the examples you gave.

  1. see a red context with only green sub-contexts, wondering how

This should not happen in the new design as siblings do not share the same context. Each sibling has its own context, which is in the new Reporter draft version the sum of its own values map and the inherited context from its parent (so the sum of all values of its lineage).

Besides, you're speaking of tree and table, why did you need that separation? Because in the drafted Reporter V2, there's so far no table, but only a tree.