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

Refactor: Replace connected nodes with edges #42

Draft
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

antonykamp
Copy link

@antonykamp antonykamp commented Apr 17, 2023

This PR replaces connected_nodes and its dependencies with connected_edges in the Node class and introduces connected_nodes as property again.
Additionally, it adds two functions to the Edge class to get the opposite node given one top node and the neighbor geo node given a top node.

fixes #40

Add get_opposite_node to calculate given a one of the top nodes the other one.
Add get_next_geo_node to calculate the neighbor geo node given one of the top nodes
Replace setting operations of connected_on_... with new connected_edge_on_... property.
Copy link

@Lietze Lietze left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job, I found one little mixup

yaramo/node.py Outdated Show resolved Hide resolved
@antonykamp antonykamp force-pushed the refactor/replace-connected-nodes-with-edges branch from 7a6def0 to 51f8a77 Compare April 18, 2023 13:29
@arneboockmeyer
Copy link
Member

@antonykamp Whats the status of this PR?

@antonykamp
Copy link
Author

I forward this to @Lietze and @SaturnHafen 😇

@SaturnHafen
Copy link

SaturnHafen commented May 30, 2023

As far as I am concerned this (and all the other PRs across the org) are ready for review. I will mark them as ready :)

Edit: I don't have permissions to mark this pr as ready

@SaturnHafen
Copy link

Also: The pyproject.toml in the other repositories reference our branches, so that might need fixing as well.

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

Successfully merging this pull request may close these issues.

Replace connected_nodes with connected_edges
4 participants