Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DotTopologySerializer duplicating exported infos #93

Open
remikey opened this issue Jan 28, 2020 · 4 comments
Open

DotTopologySerializer duplicating exported infos #93

remikey opened this issue Jan 28, 2020 · 4 comments
Assignees
Labels
Milestone

Comments

@remikey
Copy link
Member

remikey commented Jan 28, 2020

Main Issue

I think that the DotTopologySerializer exports the links twice. Although not harmful, it is not correct.

Also, in this method, we use the directed links to perform the export. Since we don't use digraph-parser anymore, I suppose DOT would allow us to use the Topology's orientation (Topology.getLinks()).

Side note

For a specific project, I recently needed to modify the serializer outputs a Link. Since the exportToString method is very monolithic, I had to copy paste the whole class and modify it. Maybe segmenting it a bit into protected methods (at least something like exportLinkToString(Link)) could help?

@remikey remikey added the bug label Jan 28, 2020
@remikey remikey added this to the next milestone Jan 28, 2020
@remikey
Copy link
Member Author

remikey commented Jan 28, 2020

@pictavien , since you have recently been the main contributor of the DotTopologySerializer, I suppose that you are the best suited to check my claims and correct what actually needs to be?

@pictavien pictavien self-assigned this Jan 28, 2020
@pictavien
Copy link
Collaborator

pictavien commented Jan 28, 2020

Ok. I take over it.
There is a bug. But, the previous version of 'exportToString', tried to handle the case where one add directed links to an undirected topology. To this purpose, it generates both kind of links (edges and arcs). When the topology is reloaded, addLink check if the arc or edge already exists, in which case the link is ignored. So we have to export 3 kinds of topology:
1/ A simple undirected topology where 2*(number of arcs) = number of edges. In this case we export only edges.
2/ A simple directed topology for which we export only arcs.
3/ An undirected topology extended with directed links.
Shall we handle this latter case ?

@remikey
Copy link
Member Author

remikey commented Jan 28, 2020

In my opinion, we are trying to support borderline cases. I think I remember discussions in which going back and forth between DIRECTED and UNDIRECTED is not officially supported. It would then be the user's responsibility to set the topology's orientation right.

Anyway, I suppose we can still do something like:

  • directed topologies
    • -> exported as DIRECTED
  • undirected topologies
    • 2*(number of arcs) == number of edges
      • -> exported as UNDIRECTED
    • 2*(number of arcs) != number of edges
      • -> exported as DIRECTED

Adding extra infos, like we used to, would not be understood by DOT parsers we don't control.

@acasteigts , your opinion on this issue ?

@pictavien
Copy link
Collaborator

For the same reason i explained above, XML Serializer also generates only directed links.
Shall I fix it in the same issue ?

@pictavien pictavien modified the milestones: v1.2.0, next Feb 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants