adds a fixed_positions param to get_network_area_diagram #928
Conversation
…agram Signed-off-by: Christian Biasuzzi <[email protected]>
pypowsybl/network/impl/network.py
Outdated
| @@ -392,7 +392,7 @@ def write_network_area_diagram(self, svg_file: PathOrStr, voltage_level_ids: Uni | |||
|
|
|||
| def get_network_area_diagram(self, voltage_level_ids: Union[str, List[str]] = None, depth: int = 0, | |||
| high_nominal_voltage_bound: float = -1, low_nominal_voltage_bound: float = -1, | |||
| nad_parameters: NadParameters = None) -> Svg: | |||
| nad_parameters: NadParameters = None, fixed_positions: str = None) -> Svg: | |||
There was a problem hiding this comment.
It would be a lot more handy to have a dataframe of positions here if we want to process the positions in python. And then a utility method to get such a dataframe from the metadata json
There was a problem hiding this comment.
we must not forget that, currently, in the json metadata are encoded the text nodes' positions , other than the nodes' positions (and could it be that, in the future, the json would contain some more graph elements?), so
a single dataframe parameter would not be enough in the get_network_area_diagram python api.
Besides, the get_network_area_diagram alredy returns a couple: the svg and the json metadata.
I wonder if it could not be better to keep it simpler, leave here the json str metadata parameter and (in the future when it is needed) write a python utility/adapter method that creates a suitable json metadata from one or more positions dataframes (and vice versa a json -> multiple dataframes converter). What do you think?
There was a problem hiding this comment.
It's clearly simpler what you suggest, but I think that having a json as one parameter among several parameters of a method is strange. Even though it's very handy in current development state.
Besides, currently the json metadata is the only thing that holds the information. So it's the easiest way to give it. But we've planned to have a specific json format which would contain all the positions (all nodes including text nodes), and only this information. If we do it now with the metadata, there will soon be a breaking change.
About the multiple dataframes, I thought that the needed information could be in a single dataframe like this (or with less columns with (x,y) pairs) :
CBiasuzzi
changed the title
Signed-off-by: Christian Biasuzzi <[email protected]>
Signed-off-by: Christian Biasuzzi <[email protected]>
Signed-off-by: Christian Biasuzzi <[email protected]>
|
|
CBiasuzzi
changed the title
| _pp.SeriesMetadata('shiftX',1,False,False,False), | ||
| _pp.SeriesMetadata('shiftY',1,False,False,False), | ||
| _pp.SeriesMetadata('connectionShiftX',1,False,False,False), | ||
| _pp.SeriesMetadata('connectionShiftY',1,False,False,False)] |
There was a problem hiding this comment.
the parameters about the legend should be more explicit, something like legendShiftX, legendConnectionShiftX?
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?
feature
What is the current behavior?
What is the new behavior (if this is a feature change)?
Adds an optional parameter fixed_positions to the get_network_area_diagram, to be able to generate a NAD with elements set to fixed positions.
Fixed_positions is a JSON encoded metadata string ( e.g., result of a previously computed NAD).
Note that positions for diagram elements that are not specified in the JSON will be computed using the current layout algorithm.
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: