Skip to content

graphs/kruskal: add a test case to verify the correctness, fix styles #2443

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

Merged
merged 7 commits into from
Oct 8, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 17 additions & 16 deletions graphs/minimum_spanning_tree_kruskal.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,5 @@
if __name__ == "__main__":
num_nodes, num_edges = list(map(int, input().strip().split()))

edges = []

for i in range(num_edges):
node1, node2, cost = list(map(int, input().strip().split()))
edges.append((i, node1, node2, cost))

edges = sorted(edges, key=lambda edge: edge[3])
def run_algorithm(num_nodes, num_edges, edges):
Copy link
Member

Choose a reason for hiding this comment

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

Please add Python type hints on function parameters and the return type.

Does num_edges always equal len(edges)? If so, let’s not force the caller to provide us num_edges.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding type hints was done in #3101 which you merged a couple of hours ago.
We can remove num_edges if you say so.

Copy link
Member

Choose a reason for hiding this comment

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

These comments were on my second computer and I never hit Send... Thanks for doing this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My pleasure 🥂

edges = sorted(edges, key=lambda edge: edge[2])

parent = list(range(num_nodes))

Expand All @@ -20,13 +12,22 @@ def find_parent(i):
minimum_spanning_tree = []

for edge in edges:
parent_a = find_parent(edge[1])
parent_b = find_parent(edge[2])
parent_a = find_parent(edge[0])
parent_b = find_parent(edge[1])
if parent_a != parent_b:
minimum_spanning_tree_cost += edge[3]
minimum_spanning_tree_cost += edge[2]
minimum_spanning_tree.append(edge)
parent[parent_a] = parent_b

print(minimum_spanning_tree_cost)
for edge in minimum_spanning_tree:
print(edge)
return minimum_spanning_tree


if __name__ == "__main__": # pragma: no cover
num_nodes, num_edges = list(map(int, input().strip().split()))
edges = []

for _ in range(num_edges):
node1, node2, cost = list(map(int, input().strip().split()))
edges.append((node1, node2, cost))

run_algorithm(num_nodes, num_edges, edges)
32 changes: 32 additions & 0 deletions graphs/tests/test_min_spanning_tree_kruskal.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
from graphs.minimum_spanning_tree_kruskal import run_algorithm as kruskal


def test_kruskal_successful_result():
num_nodes, num_edges = 9, 14
edges = [[0, 1, 4],
[0, 7, 8],
[1, 2, 8],
[7, 8, 7],
[7, 6, 1],
[2, 8, 2],
[8, 6, 6],
[2, 3, 7],
[2, 5, 4],
[6, 5, 2],
[3, 5, 14],
[3, 4, 9],
[5, 4, 10],
[1, 7, 11]]
Copy link
Member

Choose a reason for hiding this comment

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

psf/black will reformat the matrices.


result = kruskal(num_nodes, num_edges, edges)

expected = [[7, 6, 1],
[2, 8, 2],
[6, 5, 2],
[0, 1, 4],
[2, 5, 4],
[2, 3, 7],
[0, 7, 8],
[3, 4, 9]]

assert sorted(expected) == sorted(result)