-
Notifications
You must be signed in to change notification settings - Fork 51
C16 - Spruce - Vange Spracklin #38
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,105 @@ | ||
| ''' i tried to import MinHeap from the other file but it wasn't passing pytest tests | ||
|
|
||
| this is what i was writing: | ||
| from min_heap import MinHeap | ||
|
|
||
| anyway i copy and pasted the functions from previous work, but removed the Heap Node part since these tests just look at a list of numbers. | ||
|
|
||
| see line 76 for the heap_sort function! | ||
|
|
||
| ''' | ||
|
|
||
| class MinHeap: | ||
|
|
||
| def __init__(self): | ||
| self.store = [] | ||
|
|
||
| def add(self, key): | ||
| self.store.append(key) | ||
| last_index = len(self.store) - 1 | ||
| self.heap_up(last_index) | ||
| return | ||
|
|
||
| def remove(self): | ||
| if self.empty(): | ||
| return | ||
| self.swap(0, -1) | ||
| result = self.store.pop() | ||
| if not self.empty(): | ||
| self.heap_down(0) | ||
| return result | ||
|
|
||
| def empty(self): | ||
| if len(self.store) == 0: | ||
| return True | ||
| return False | ||
|
|
||
| def heap_up(self, index): | ||
| parent_index = int((index - 1) / 2) | ||
| current = self.store[index] | ||
| parent = self.store[parent_index] | ||
|
|
||
| while current < parent: | ||
| self.swap(index, parent_index) | ||
| index = parent_index | ||
| parent_index = int((index - 1) / 2) | ||
| current = self.store[index] | ||
| parent = self.store[parent_index] | ||
|
|
||
| def has_left_child(self, index): | ||
| left_child_index = int((2 * index) + 1) | ||
| if left_child_index <= len(self.store) - 1: | ||
| return True | ||
| return False | ||
|
|
||
| def has_right_child(self, index): | ||
| right_child_index = int((2 * index) + 2) | ||
| if right_child_index <= len(self.store) - 1: | ||
| return True | ||
| return False | ||
|
|
||
| def heap_down(self, index): | ||
| if self.has_left_child(index) and (self.store[index] > self.store[int((2 * index) + 1)]): | ||
| left_child_index = (2 * index) + 1 | ||
| self.swap(index, left_child_index) | ||
| self.heap_down(left_child_index) | ||
|
|
||
| if self.has_right_child(index) and (self.store[index] > self.store[int((2 * index) + 2)]): | ||
| right_child_index = (2 * index) + 2 | ||
| self.swap(index, right_child_index) | ||
| self.heap_down(right_child_index) | ||
| return | ||
|
|
||
| def swap(self, index_1, index_2): | ||
| temp = self.store[index_1] | ||
| self.store[index_1] = self.store[index_2] | ||
| self.store[index_2] = temp | ||
|
|
||
| def heap_sort(list): | ||
| """ This method uses a heap to sort an array. | ||
| Time Complexity: ? | ||
| Space Complexity: ? | ||
| Time Complexity: ? O(n) | ||
| Space Complexity: ? O(n) | ||
|
Comment on lines
+80
to
+81
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👀 The theoretical best time complexity for a general sorting algorithm (like heap sort) is O(n log n). Here, we can see that we add n things to the heap (which each take O(log n)), then we remove n things from the heap, which again takes O(log n) each, for a total of O(2 n log n) → O(n log n). The space complexity is O(n) due to the internal store that |
||
| """ | ||
| pass | ||
| result = [] | ||
| if not list: | ||
| return result | ||
|
|
||
| da_heap = MinHeap() | ||
|
|
||
| for num in list: | ||
| da_heap.add(num) | ||
|
|
||
| while len(result) != len(list): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider using the while not da_heap.empty(): |
||
| top_of_heap = da_heap.remove() | ||
| result.append(top_of_heap) | ||
|
|
||
| return result | ||
|
|
||
|
|
||
| nums = [5, 27, 3, 16, 50] | ||
| print(f"{nums} <---given nums") | ||
| result = heap_sort(nums) | ||
| print(f"{result} <---result of heap_sort") | ||
| expected = [3, 5, 16, 27, 50] | ||
| print(f"{expected} <--- expected result") | ||
| print(f"do they match? {result == expected}") | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,20 +19,29 @@ def __init__(self): | |
| def add(self, key, value = None): | ||
| """ This method adds a HeapNode instance to the heap | ||
| If value == None the new node's value should be set to key | ||
| Time Complexity: ? | ||
| Space Complexity: ? | ||
| Time Complexity: ? O(log n) | ||
| Space Complexity: ? O(1) | ||
|
Comment on lines
+22
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✨ Great! In the worst case, the new value we're inserting is the new root of the heap, meaning it would need to move up the full height of the heap (which is log n levels deep). Your implementation of the |
||
| """ | ||
| pass | ||
| if value == None: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👀 Prefer using |
||
| value = key | ||
| self.store.append(HeapNode(key, value)) | ||
| last_index = len(self.store) - 1 | ||
| self.heap_up(last_index) | ||
| return | ||
|
|
||
| def remove(self): | ||
| """ This method removes and returns an element from the heap | ||
| maintaining the heap structure | ||
| Time Complexity: ? | ||
| Space Complexity: ? | ||
| Time Complexity: ? O(log n) | ||
| Space Complexity: ? O(1) | ||
|
Comment on lines
+35
to
+36
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As the cost of |
||
| """ | ||
| pass | ||
|
|
||
|
|
||
| if self.empty(): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✨ Nice use of your own helper method! |
||
| return | ||
| self.swap(0, -1) | ||
| result = self.store.pop() | ||
| if not self.empty(): | ||
| self.heap_down(0) | ||
|
Comment on lines
+42
to
+43
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Calling |
||
| return str(result) | ||
|
|
||
| def __str__(self): | ||
| """ This method lets you print the heap, when you're testing your app. | ||
|
|
@@ -44,11 +53,12 @@ def __str__(self): | |
|
|
||
| def empty(self): | ||
| """ This method returns true if the heap is empty | ||
| Time complexity: ? | ||
| Space complexity: ? | ||
| Time complexity: ? O(1) | ||
| Space complexity: ? O(1) | ||
|
Comment on lines
+56
to
+57
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✨ |
||
| """ | ||
| pass | ||
|
|
||
| if len(self.store) == 0: | ||
| return True | ||
| return False | ||
|
Comment on lines
+59
to
+61
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remember that an empty list is falsy return not self.store |
||
|
|
||
| def heap_up(self, index): | ||
| """ This helper method takes an index and | ||
|
|
@@ -57,20 +67,51 @@ def heap_up(self, index): | |
| property is reestablished. | ||
|
|
||
| This could be **very** helpful for the add method. | ||
| Time complexity: ? | ||
| Space complexity: ? | ||
| Time complexity: ? O(log n) | ||
| Space complexity: ? O(1) | ||
|
Comment on lines
+70
to
+71
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✨ Yes, this function is where the complexity in |
||
| """ | ||
| pass | ||
| parent_index = int((index - 1) / 2) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider using truncating division parent_index = (index - 1) // 2 |
||
| current_key = self.store[index].key | ||
| parent_key = self.store[parent_index].key | ||
|
|
||
| while current_key < parent_key: | ||
| self.swap(index, parent_index) | ||
| index = parent_index | ||
| parent_index = int((index - 1) / 2) | ||
| current_key = self.store[index].key | ||
| parent_key = self.store[parent_index].key | ||
|
Comment on lines
+77
to
+82
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✨ Nice use of iteration to avoid incurring extra space complexity. Notice that this code ends up repeating the calculations for while index > 0:
parent_index = (index - 1) // 2
current_key = self.store[index].key
parent_key = self.store[parent_index].key
if current_key < parent_key:
self.swap(index, parent_index)
index = parent_index |
||
|
|
||
| def has_left_child(self, index): | ||
| left_child_index = int((2 * index) + 1) | ||
| if left_child_index <= len(self.store) - 1: | ||
| return True | ||
| return False | ||
|
Comment on lines
+86
to
+88
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👀 Performing a boolean check (if condition) then returning a boolean is an anti-pattern. We can return the value of the condition directly. return left_child_index <= len(self.store) - 1 |
||
|
|
||
| def has_right_child(self, index): | ||
| right_child_index = int((2 * index) + 2) | ||
| if right_child_index <= len(self.store) - 1: | ||
| return True | ||
| return False | ||
|
|
||
| def heap_down(self, index): | ||
| """ This helper method takes an index and | ||
| moves the corresponding element down the heap if it's | ||
| larger than either of its children and continues until | ||
| the heap property is reestablished. | ||
| time complexity O(log n) | ||
| space complexity O(1) | ||
|
Comment on lines
+101
to
+102
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👀 Nice of you to include the complexity here when we forgot to ask for it! This is where the O(log n) time and space complexity in |
||
| """ | ||
| pass | ||
| if self.has_left_child(index) and (self.store[index].key > self.store[int((2 * index) + 1)].key): | ||
| left_child_index = (2 * index) + 1 | ||
| self.swap(index, left_child_index) | ||
| self.heap_down(left_child_index) | ||
|
Comment on lines
+106
to
+107
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Before deciding to swap here, we would also need to compare the left child to the right (if present). We only want to swap the one child which is the smallest. Currently, if the left child were smaller then the parent, but larger than the right, first we would continue swapping the parent down the left subtree, then potentially need to swap the new parent down the right sub tree. Consider the arrangement [3, 2, 1]. 3 > 2 so it would swap with the left child → [2, 3, 1]. But notice the heap property is not re-established since 2 > 1. This code will continue and swap 2 and 1 → [1, 3, 2]. But if we had compare the values of the children first, we could see that 1 < 2 and swap that with 3 directly → [1, 2, 3] which re-establishes the heap in a single swap. |
||
|
|
||
|
|
||
| if self.has_right_child(index) and (self.store[index].key > self.store[int((2 * index) + 2)].key): | ||
| right_child_index = (2 * index) + 2 | ||
| self.swap(index, right_child_index) | ||
| self.heap_down(right_child_index) | ||
| return | ||
|
|
||
| def swap(self, index_1, index_2): | ||
| """ Swaps two elements in self.store | ||
| at index_1 and index_2 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -69,12 +69,12 @@ def test_it_can_remove_nodes_in_proper_order(heap): | |
| heap.add(0, "Donuts") | ||
| heap.add(16, "Cookies") | ||
|
|
||
|
|
||
| # Act | ||
| returned_items = ["Donuts", "Pizza", "Pasta", "Soup", "Cookies", "Cake"] | ||
| # assert heap.store == returned_items | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can't make a check like this since the store is a list of heap nodes, not just the values. We also wouldn't generally expect the order of the items being returned to match the final retrieved order, since a heap is only loosely ordered. We only know that the smallest (or largest if a max heap) value is at the front, but we can't say with certainty the specific ordering of the remaining items. |
||
|
|
||
| for item in returned_items: | ||
| assert heap.remove() == item | ||
|
|
||
| def test_removing_a_node_from_an_empty_heap_is_none(heap): | ||
| assert heap.remove() == None | ||
| assert heap.remove() == None | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could either do
or