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

Fixed the error about edge_attr dict in cases where edge is defined a… #254

Merged

Conversation

neserende
Copy link
Contributor

…s (large num, small num) instead of (small num, large num)

Identify the Bug

Issue: #253

Description of the Change

As it is stated in the issue, when edges are not defined like (small number, large number), attribute related to it cannot be found in the edge_attr dictionary. So whenever we try to access a value from that dictionary, we should also check if (large number, small number) version of the edge exists. Not doing this check results in a key error.

Note: In directed graphs, this change does not apply. Therefore, in EdgeCategorical compartment, we also introduce a flag which is True when the graph is directed. This extra check in edge_attr is done when the graph is undirected.

Possible Drawbacks

In my tests, I haven't seen any negative impacts or bugs caused by this change. Directed graphs can be inspected further.

Verification Process

I have tested with different type of networks generated by networkx library (erdos_renyi_graph, watts_strogatz_graph, random_regular_graph, barabasi_albert_graph) as well as a file input. All graphs were undirected. By inspecting the iteration trends, we can observe that it works as intended.

Release Notes

Edge attribute compartments now support undirected edges defined as (large num, small num)

…s (large num, small num) instead of (small num, large num)
@GiulioRossetti GiulioRossetti merged commit aeda373 into GiulioRossetti:master Mar 26, 2024
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.

2 participants