feat(algorithms, heaps): top k and kth largest #152
Conversation
BrianLusina
added
enhancement
Algorithm
📝 WalkthroughWalkthroughAdded two new heap algorithm features: Top K Largest (k_largest, kth_largest, kth_largest_sorting) and K Closest Points to Origin (heap and sorting implementations), plus documentation updates and unit tests for both features. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@algorithms/heap/topklargest/__init__.py`:
- Around line 72-76: The in-place call nums.sort(reverse=True) mutates the
caller's list; change it to operate on a non-mutating sorted sequence (e.g., use
sorted(nums, reverse=True) or sort a shallow copy) and then return the k-th
largest from that result (replace the nums.sort and return nums[k - 1] usage
with a non-mutating sorted result variable and return that_result[k - 1]);
ensure references to nums.sort(reverse=True) and the subsequent return are
updated accordingly.
- Around line 5-27: k_largest lacks input validation and can IndexError on empty
inputs or invalid k; add guards at the start of k_largest to validate params
(nums and k) consistent with the other variants: if not nums or k <= 0 or k >
len(nums) raise a ValueError with a clear message (e.g., "k must be between 1
and len(nums) and nums must be non-empty"); perform this validation before
building min_heap and iterating.
🧹 Nitpick comments (2)
algorithms/heap/topklargest/__init__.py (1)
30-56: Implementation is correct; consider exception vs sentinel for invalid input.The heap-based algorithm is correctly implemented with O(n log k) time complexity. One consideration: returning
-1for invalid input could be ambiguous if-1is a valid element in the array. An alternative would be to raise aValueError, which is more Pythonic for invalid arguments. However, this is consistent withkth_largest_sorting, so the current approach is acceptable if documented.algorithms/heap/topklargest/test_top_k_largest_elements.py (1)
6-27: Consider adding edge case tests for invalid inputs.The current test cases cover happy paths well, but consider adding tests for:
- Empty list:
([], 1, -1)forkth_largest/kth_largest_sortingk=0:([1,2,3], 0, -1)k > len(nums):([1,2], 5, -1)This would validate the input validation logic and document expected behavior.
📝 Suggested additional test cases
KTH_LARGEST_EDGE_CASES = [ ([], 1, -1), # empty list ([1, 2, 3], 0, -1), # k = 0 ([1, 2], 5, -1), # k > len(nums) ]
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (16)
algorithms/heap/topklargest/images/solutions/kth_largest_element_in_array_solution_1.pngis excluded by!**/*.pngalgorithms/heap/topklargest/images/solutions/kth_largest_element_in_array_solution_10.pngis excluded by!**/*.pngalgorithms/heap/topklargest/images/solutions/kth_largest_element_in_array_solution_11.pngis excluded by!**/*.pngalgorithms/heap/topklargest/images/solutions/kth_largest_element_in_array_solution_2.pngis excluded by!**/*.pngalgorithms/heap/topklargest/images/solutions/kth_largest_element_in_array_solution_3.pngis excluded by!**/*.pngalgorithms/heap/topklargest/images/solutions/kth_largest_element_in_array_solution_4.pngis excluded by!**/*.pngalgorithms/heap/topklargest/images/solutions/kth_largest_element_in_array_solution_5.pngis excluded by!**/*.pngalgorithms/heap/topklargest/images/solutions/kth_largest_element_in_array_solution_6.pngis excluded by!**/*.pngalgorithms/heap/topklargest/images/solutions/kth_largest_element_in_array_solution_7.pngis excluded by!**/*.pngalgorithms/heap/topklargest/images/solutions/kth_largest_element_in_array_solution_8.pngis excluded by!**/*.pngalgorithms/heap/topklargest/images/solutions/kth_largest_element_in_array_solution_9.pngis excluded by!**/*.pngalgorithms/heap/topklargest/images/solutions/top_k_largest_element_in_array_solution_1.pngis excluded by!**/*.pngalgorithms/heap/topklargest/images/solutions/top_k_largest_element_in_array_solution_2.pngis excluded by!**/*.pngalgorithms/heap/topklargest/images/solutions/top_k_largest_element_in_array_solution_3.pngis excluded by!**/*.pngalgorithms/heap/topklargest/images/solutions/top_k_largest_element_in_array_solution_4.pngis excluded by!**/*.pngalgorithms/heap/topklargest/images/solutions/top_k_largest_element_in_array_solution_5.pngis excluded by!**/*.png
📒 Files selected for processing (6)
DIRECTORY.mdalgorithms/heap/topkfrequentelements/README.mdalgorithms/heap/topkfrequentelements/__init__.pyalgorithms/heap/topklargest/README.mdalgorithms/heap/topklargest/__init__.pyalgorithms/heap/topklargest/test_top_k_largest_elements.py
🧰 Additional context used
🧬 Code graph analysis (1)
algorithms/heap/topklargest/test_top_k_largest_elements.py (1)
algorithms/heap/topklargest/__init__.py (3)
k_largest(5-27)kth_largest(30-56)kth_largest_sorting(59-76)
🪛 LanguageTool
algorithms/heap/topklargest/README.md
[style] ~84-~84: Using many exclamation marks might seem excessive (in this case: 11 exclamation marks for a text that’s 3674 characters long)
Context: ...argest_element_in_array_solution_1.png) 
🪛 markdownlint-cli2 (0.18.1)
DIRECTORY.md
143-143: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
144-144: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
🔇 Additional comments (4)
algorithms/heap/topklargest/README.md (2)
1-46: Documentation is well-structured and accurate.The explanation of the min-heap approach for Top-K elements is clear, with appropriate complexity analysis. The note about variable k complexity is helpful for understanding the general case.
49-106: LGTM!The documentation for the Kth Largest Element problem is comprehensive, covering both sorting and min-heap approaches with accurate complexity analysis.
algorithms/heap/topklargest/test_top_k_largest_elements.py (1)
12-16: LGTM!Good use of
sorted()comparison fork_largestsince the heap doesn't guarantee order within the result set.DIRECTORY.md (1)
143-144: LGTM!The new entry for
Topklargestfollows the existing indentation pattern in the file. The static analysis warning about indentation can be safely ignored as the style is consistent with other entries in the Heap section (e.g., Schedule Tasks on lines 141-142).
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| def k_largest(nums: List[int], k: int = 3) -> List[int]: | ||
| """ | ||
| Finds the k largest elements in a given list. K is defaulted to three, but can be used to tweak the k largest | ||
| elements in the array | ||
| Args: | ||
| nums(list): list of elements to check for | ||
| k(int): number of elements to check, defaulted to 3 | ||
| Returns: | ||
| list: top k largest elements | ||
| """ | ||
| # create a minimum heap with the first k elements | ||
| min_heap = nums[:k] | ||
| heapq.heapify(min_heap) | ||
|
|
||
| # iterate through the remaining elements | ||
| for num in nums[k:]: | ||
| # if the current number is greater than the element at the top of the heap | ||
| if num > min_heap[0]: | ||
| # Remove it and add this element | ||
| heapq.heappushpop(min_heap, num) | ||
|
|
||
| # return the top k elements | ||
| return min_heap |
There was a problem hiding this comment.
Missing input validation can cause runtime errors.
Unlike kth_largest and kth_largest_sorting, this function lacks input validation. This will cause:
IndexErrorwhenk=0(empty heap, then accessingmin_heap[0])IndexErrorwhennumsis empty- Unexpected behavior when
k > len(nums)
Consider adding consistent validation:
🛠️ Suggested fix
def k_largest(nums: List[int], k: int = 3) -> List[int]:
"""
Finds the k largest elements in a given list. K is defaulted to three, but can be used to tweak the k largest
elements in the array
Args:
nums(list): list of elements to check for
k(int): number of elements to check, defaulted to 3
Returns:
list: top k largest elements
"""
+ # input validation to ensure we don't get unexpected results
+ if not nums or k <= 0:
+ return []
+
+ # Adjust k if it exceeds the length of nums
+ k = min(k, len(nums))
+
# create a minimum heap with the first k elements
min_heap = nums[:k]
heapq.heapify(min_heap)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def k_largest(nums: List[int], k: int = 3) -> List[int]: | |
| """ | |
| Finds the k largest elements in a given list. K is defaulted to three, but can be used to tweak the k largest | |
| elements in the array | |
| Args: | |
| nums(list): list of elements to check for | |
| k(int): number of elements to check, defaulted to 3 | |
| Returns: | |
| list: top k largest elements | |
| """ | |
| # create a minimum heap with the first k elements | |
| min_heap = nums[:k] | |
| heapq.heapify(min_heap) | |
| # iterate through the remaining elements | |
| for num in nums[k:]: | |
| # if the current number is greater than the element at the top of the heap | |
| if num > min_heap[0]: | |
| # Remove it and add this element | |
| heapq.heappushpop(min_heap, num) | |
| # return the top k elements | |
| return min_heap | |
| def k_largest(nums: List[int], k: int = 3) -> List[int]: | |
| """ | |
| Finds the k largest elements in a given list. K is defaulted to three, but can be used to tweak the k largest | |
| elements in the array | |
| Args: | |
| nums(list): list of elements to check for | |
| k(int): number of elements to check, defaulted to 3 | |
| Returns: | |
| list: top k largest elements | |
| """ | |
| # input validation to ensure we don't get unexpected results | |
| if not nums or k <= 0: | |
| return [] | |
| # Adjust k if it exceeds the length of nums | |
| k = min(k, len(nums)) | |
| # create a minimum heap with the first k elements | |
| min_heap = nums[:k] | |
| heapq.heapify(min_heap) | |
| # iterate through the remaining elements | |
| for num in nums[k:]: | |
| # if the current number is greater than the element at the top of the heap | |
| if num > min_heap[0]: | |
| # Remove it and add this element | |
| heapq.heappushpop(min_heap, num) | |
| # return the top k elements | |
| return min_heap |
🤖 Prompt for AI Agents
In `@algorithms/heap/topklargest/__init__.py` around lines 5 - 27, k_largest lacks
input validation and can IndexError on empty inputs or invalid k; add guards at
the start of k_largest to validate params (nums and k) consistent with the other
variants: if not nums or k <= 0 or k > len(nums) raise a ValueError with a clear
message (e.g., "k must be between 1 and len(nums) and nums must be non-empty");
perform this validation before building min_heap and iterating.
| # Sort the list in place which incurs a time complexity cost of O(n log(n)). Space complexity is O(n) due to using | ||
| # an in-memory data structure to store the elements during sorting using timsort in Python | ||
| nums.sort(reverse=True) | ||
| # Return the kth largest element | ||
| return nums[k - 1] |
There was a problem hiding this comment.
In-place sort mutates the input list unexpectedly.
The nums.sort(reverse=True) call modifies the caller's list, which may be an unintended side effect. This could cause subtle bugs if the caller expects nums to remain unchanged.
Consider using sorted() to avoid mutation:
🛠️ Suggested fix
- # Sort the list in place which incurs a time complexity cost of O(n log(n)). Space complexity is O(n) due to using
- # an in-memory data structure to store the elements during sorting using timsort in Python
- nums.sort(reverse=True)
+ # Sort the list which incurs a time complexity cost of O(n log(n)). Space complexity is O(n) due to creating
+ # a new sorted list
+ sorted_nums = sorted(nums, reverse=True)
# Return the kth largest element
- return nums[k - 1]
+ return sorted_nums[k - 1]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Sort the list in place which incurs a time complexity cost of O(n log(n)). Space complexity is O(n) due to using | |
| # an in-memory data structure to store the elements during sorting using timsort in Python | |
| nums.sort(reverse=True) | |
| # Return the kth largest element | |
| return nums[k - 1] | |
| # Sort the list which incurs a time complexity cost of O(n log(n)). Space complexity is O(n) due to creating | |
| # a new sorted list | |
| sorted_nums = sorted(nums, reverse=True) | |
| # Return the kth largest element | |
| return sorted_nums[k - 1] |
🤖 Prompt for AI Agents
In `@algorithms/heap/topklargest/__init__.py` around lines 72 - 76, The in-place
call nums.sort(reverse=True) mutates the caller's list; change it to operate on
a non-mutating sorted sequence (e.g., use sorted(nums, reverse=True) or sort a
shallow copy) and then return the k-th largest from that result (replace the
nums.sort and return nums[k - 1] usage with a non-mutating sorted result
variable and return that_result[k - 1]); ensure references to
nums.sort(reverse=True) and the subsequent return are updated accordingly.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@algorithms/heap/topkclosesttoorigin/__init__.py`:
- Around line 31-36: k_closest_to_origin_sorting currently mutates the input by
calling points.sort(); change it to avoid side effects by sorting a copy (use
sorted(points, key=...)) or by copying first (points_copy = list(points);
points_copy.sort(...)) and then return the first k from the sorted copy so the
original input remains unchanged, matching the non-mutating behavior of
k_closest_to_origin.
In `@algorithms/heap/topkclosesttoorigin/README.md`:
- Around line 6-8: The distance formula in the README is garbled; replace the
incorrect expression with correct mathematical notation: for distance from
origin use "sqrt(x² + y²)" (or "√(x² + y²)"), and for distance between two
points use "sqrt((x₁ - x₂)² + (y₁ - y₂)²)" (or "√((x₁ - x₂)² + (y₁ - y₂)²)"),
ensuring variable names match those used elsewhere in the doc (e.g., x,y and a,b
or x₁,y₁ and x₂,y₂) and remove the malformed `√(x1 - a2)2 + (y1 - b2)2` text.
In `@algorithms/heap/topkclosesttoorigin/test_top_k_closest_to_origin.py`:
- Around line 23-25: The test sorts results only by x-coordinate which can be
flaky when x values tie; update the sorting key in
test_top_k_closest_to_origin.py to use both coordinates (e.g., key that returns
a tuple of (x, y)) for sorted_expected and sorted_actual in the block using
sorted_expected/sorted_actual and apply the same change in
test_top_k_closest_to_origin_sorting so comparisons are deterministic.
🧹 Nitpick comments (1)
algorithms/heap/topkclosesttoorigin/__init__.py (1)
5-28: Consider adding edge case validation.The heap-based implementation is correct and efficient. However, there's no validation for edge cases:
k <= 0would return an empty list (acceptable but undocumented)k > len(points)would return all points (acceptable)- Empty
pointslist would return empty (acceptable)These behaviors may be intentional, but documenting them or adding explicit validation would improve robustness.
🔧 Optional: Add input validation
def k_closest_to_origin(points: List[List[int]], k: int) -> List[List[int]]: + if not points or k <= 0: + return [] + k = min(k, len(points)) + # Max heap will store the top k points closest to the origin in the form (-distance, idx). The distance is negated
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (13)
algorithms/heap/topkclosesttoorigin/images/examples/top_k_closest_to_origin_example_1.pngis excluded by!**/*.pngalgorithms/heap/topkclosesttoorigin/images/solutions/top_k_closest_to_origin_solution_0.pngis excluded by!**/*.pngalgorithms/heap/topkclosesttoorigin/images/solutions/top_k_closest_to_origin_solution_1.pngis excluded by!**/*.pngalgorithms/heap/topkclosesttoorigin/images/solutions/top_k_closest_to_origin_solution_10.pngis excluded by!**/*.pngalgorithms/heap/topkclosesttoorigin/images/solutions/top_k_closest_to_origin_solution_11.pngis excluded by!**/*.pngalgorithms/heap/topkclosesttoorigin/images/solutions/top_k_closest_to_origin_solution_2.pngis excluded by!**/*.pngalgorithms/heap/topkclosesttoorigin/images/solutions/top_k_closest_to_origin_solution_3.pngis excluded by!**/*.pngalgorithms/heap/topkclosesttoorigin/images/solutions/top_k_closest_to_origin_solution_4.pngis excluded by!**/*.pngalgorithms/heap/topkclosesttoorigin/images/solutions/top_k_closest_to_origin_solution_5.pngis excluded by!**/*.pngalgorithms/heap/topkclosesttoorigin/images/solutions/top_k_closest_to_origin_solution_6.pngis excluded by!**/*.pngalgorithms/heap/topkclosesttoorigin/images/solutions/top_k_closest_to_origin_solution_7.pngis excluded by!**/*.pngalgorithms/heap/topkclosesttoorigin/images/solutions/top_k_closest_to_origin_solution_8.pngis excluded by!**/*.pngalgorithms/heap/topkclosesttoorigin/images/solutions/top_k_closest_to_origin_solution_9.pngis excluded by!**/*.png
📒 Files selected for processing (4)
DIRECTORY.mdalgorithms/heap/topkclosesttoorigin/README.mdalgorithms/heap/topkclosesttoorigin/__init__.pyalgorithms/heap/topkclosesttoorigin/test_top_k_closest_to_origin.py
🧰 Additional context used
🧬 Code graph analysis (1)
algorithms/heap/topkclosesttoorigin/test_top_k_closest_to_origin.py (1)
algorithms/heap/topkclosesttoorigin/__init__.py (2)
k_closest_to_origin(5-28)k_closest_to_origin_sorting(31-36)
🪛 LanguageTool
algorithms/heap/topkclosesttoorigin/README.md
[style] ~82-~82: Using many exclamation marks might seem excessive (in this case: 10 exclamation marks for a text that’s 3221 characters long)
Context: ...top_k_closest_to_origin_solution_0.png) 
🪛 markdownlint-cli2 (0.18.1)
DIRECTORY.md
143-143: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
144-144: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
145-145: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
146-146: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
🔇 Additional comments (3)
DIRECTORY.md (1)
143-146: LGTM!The new directory entries for
TopkclosesttooriginandTopklargestare correctly added and follow the existing indentation pattern used throughout the file. The static analysis hints about indentation (MD007) are false positives since the file consistently uses 4-space and 6-space indentation for nested list items.algorithms/heap/topkclosesttoorigin/README.md (1)
56-104: LGTM!The algorithm explanation and complexity analysis are well-written and accurate. The cross-reference to the related
topklargestimplementation provides helpful context.algorithms/heap/topkclosesttoorigin/test_top_k_closest_to_origin.py (1)
9-13: Test coverage is adequate for core functionality.The test cases cover the basic scenarios. Consider adding edge cases in future iterations:
k = 0k > len(points)- Points with identical distances to origin
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| def k_closest_to_origin_sorting(points: List[List[int]], k: int) -> List[List[int]]: | ||
| # Sort the points by the distance from the origin. This incurs a cost of O(n log(n)) and space cost of O(n) due to | ||
| # timsort | ||
| points.sort(key=lambda p: p[0] ** 2 + p[1] ** 2) | ||
| # Retrieve the top k points closest to the origin | ||
| return points[:k] |
There was a problem hiding this comment.
Input list is mutated unexpectedly.
points.sort() modifies the input list in-place. This is a side effect that differs from k_closest_to_origin (which preserves the input). Callers may not expect their original list to be reordered after calling this function.
🐛 Proposed fix to avoid mutating input
def k_closest_to_origin_sorting(points: List[List[int]], k: int) -> List[List[int]]:
# Sort the points by the distance from the origin. This incurs a cost of O(n log(n)) and space cost of O(n) due to
# timsort
- points.sort(key=lambda p: p[0] ** 2 + p[1] ** 2)
- # Retrieve the top k points closest to the origin
- return points[:k]
+ sorted_points = sorted(points, key=lambda p: p[0] ** 2 + p[1] ** 2)
+ # Retrieve the top k points closest to the origin
+ return sorted_points[:k]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def k_closest_to_origin_sorting(points: List[List[int]], k: int) -> List[List[int]]: | |
| # Sort the points by the distance from the origin. This incurs a cost of O(n log(n)) and space cost of O(n) due to | |
| # timsort | |
| points.sort(key=lambda p: p[0] ** 2 + p[1] ** 2) | |
| # Retrieve the top k points closest to the origin | |
| return points[:k] | |
| def k_closest_to_origin_sorting(points: List[List[int]], k: int) -> List[List[int]]: | |
| # Sort the points by the distance from the origin. This incurs a cost of O(n log(n)) and space cost of O(n) due to | |
| # timsort | |
| sorted_points = sorted(points, key=lambda p: p[0] ** 2 + p[1] ** 2) | |
| # Retrieve the top k points closest to the origin | |
| return sorted_points[:k] |
🤖 Prompt for AI Agents
In `@algorithms/heap/topkclosesttoorigin/__init__.py` around lines 31 - 36,
k_closest_to_origin_sorting currently mutates the input by calling
points.sort(); change it to avoid side effects by sorting a copy (use
sorted(points, key=...)) or by copying first (points_copy = list(points);
points_copy.sort(...)) and then return the first k from the sorted copy so the
original input remains unchanged, matching the non-mutating behavior of
k_closest_to_origin.
| The distance between two points (x, y) and (a, b) is calculated using the formula: | ||
|
|
||
| √(x1 - a2)2 + (y1 - b2)2 |
There was a problem hiding this comment.
Distance formula notation is incorrect.
The formula √(x1 - a2)2 + (y1 - b2)2 appears garbled. For distance from origin (0, 0), it should be:
√(x² + y²) or equivalently sqrt(x² + y²)
For general distance between two points (x₁, y₁) and (x₂, y₂):
√((x₁ - x₂)² + (y₁ - y₂)²)
📝 Suggested fix
The distance between two points (x, y) and (a, b) is calculated using the formula:
-√(x1 - a2)2 + (y1 - b2)2
+√((x - a)² + (y - b)²)
+
+For distance from the origin (0, 0), this simplifies to: √(x² + y²)🤖 Prompt for AI Agents
In `@algorithms/heap/topkclosesttoorigin/README.md` around lines 6 - 8, The
distance formula in the README is garbled; replace the incorrect expression with
correct mathematical notation: for distance from origin use "sqrt(x² + y²)" (or
"√(x² + y²)"), and for distance between two points use "sqrt((x₁ - x₂)² + (y₁ -
y₂)²)" (or "√((x₁ - x₂)² + (y₁ - y₂)²)"), ensuring variable names match those
used elsewhere in the doc (e.g., x,y and a,b or x₁,y₁ and x₂,y₂) and remove the
malformed `√(x1 - a2)2 + (y1 - b2)2` text.
| sorted_expected = sorted(expected, key=lambda x: x[0]) | ||
| sorted_actual = sorted(actual, key=lambda x: x[0]) | ||
| self.assertEqual(sorted_expected, sorted_actual) |
There was a problem hiding this comment.
Sorting key may cause flaky comparisons.
Sorting only by x[0] could produce non-deterministic results when two points share the same x-coordinate. For robust comparison, sort by both coordinates.
🐛 Proposed fix
- sorted_expected = sorted(expected, key=lambda x: x[0])
- sorted_actual = sorted(actual, key=lambda x: x[0])
+ sorted_expected = sorted(expected, key=lambda x: (x[0], x[1]))
+ sorted_actual = sorted(actual, key=lambda x: (x[0], x[1]))The same fix should be applied to test_top_k_closest_to_origin_sorting at lines 33-34.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| sorted_expected = sorted(expected, key=lambda x: x[0]) | |
| sorted_actual = sorted(actual, key=lambda x: x[0]) | |
| self.assertEqual(sorted_expected, sorted_actual) | |
| sorted_expected = sorted(expected, key=lambda x: (x[0], x[1])) | |
| sorted_actual = sorted(actual, key=lambda x: (x[0], x[1])) | |
| self.assertEqual(sorted_expected, sorted_actual) |
🤖 Prompt for AI Agents
In `@algorithms/heap/topkclosesttoorigin/test_top_k_closest_to_origin.py` around
lines 23 - 25, The test sorts results only by x-coordinate which can be flaky
when x values tie; update the sorting key in test_top_k_closest_to_origin.py to
use both coordinates (e.g., key that returns a tuple of (x, y)) for
sorted_expected and sorted_actual in the block using
sorted_expected/sorted_actual and apply the same change in
test_top_k_closest_to_origin_sorting so comparisons are deterministic.
Describe your change:
Checklist:
Fixes: #{$ISSUE_NO}.Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.