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

Replace usage of connected_nodes during the refactoring #3

Open
4 tasks
antonykamp opened this issue Apr 13, 2023 · 2 comments
Open
4 tasks

Replace usage of connected_nodes during the refactoring #3

antonykamp opened this issue Apr 13, 2023 · 2 comments

Comments

@antonykamp
Copy link

During the refactoring See simulate-digital-rail/yaramo#40 we have to replace writing access of connected_nodes with connected_edges and edit calls of set_connection_left, set_connection_right and set_connection_head to take edges instead of nodes.

Acceptance criterion

  • Replace writing access of connected_nodes with connected_edges
  • Give set_connection_left the edge instead of a node
  • Give set_connection_right the edge instead of a node
  • Give set_connection_head the edge instead of a node
@antonykamp
Copy link
Author

I was trying to implement the refactored version of Yaramo. But I couldn't figure the example out:
https://github.com/simulate-digital-rail/interlocking-exporter/blob/main/examples/example.py
Please verify that the graph at the top of the file equals the code. 🙃
Thank you!

@Lietze
Copy link

Lietze commented Apr 18, 2023

I think from line 36 to 38 the connections of node d are being set again. Before, it seems to me like the graph is actuate, but this changes where d is. These are the lines:

    node_d.set_connection_left(node_c)
    node_d.set_connection_right(node_f)
    node_d.set_connection_head(node_e)

It also seems to me like there are way too many edges. I think you just have to radically cut out many of them. One edge for each connection should be enough. If that does not work, I would also try to keep edges for both directions, even though I am relatively sure that Yaramo does not need that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants