feat(puzzles, math): rectangle area and max consecutive ones #117
Conversation
WalkthroughAdds a new Rectangle Area feature under pymath (implementation, README, tests) and introduces a max-consecutive-ones function with README updates and parameterized tests for the puzzles/arrays collection. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
DIRECTORY.md(1 hunks)pymath/rectangle_area/README.md(1 hunks)pymath/rectangle_area/__init__.py(1 hunks)pymath/rectangle_area/test_compute_area.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
pymath/rectangle_area/test_compute_area.py (1)
pymath/rectangle_area/__init__.py (1)
compute_area(1-33)
🪛 markdownlint-cli2 (0.18.1)
DIRECTORY.md
707-707: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
708-708: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🔇 Additional comments (1)
pymath/rectangle_area/test_compute_area.py (1)
1-33: Add type hintexpected: intfor parameter consistency.The
parameterizeddependency is already declared inpyproject.toml(version^0.9.0), so no verification is needed. However, add the type hint to match the pattern used in similar test files across the codebase:- expected, + expected: int,
| def compute_area( | ||
| ax1: int, ay1: int, ax2: int, ay2: int, bx1: int, by1: int, bx2: int, by2: int | ||
| ): | ||
| # 1. Calculate the area of each individual rectangle | ||
| area_a = (ax2 - ax1) * (ay2 - ay1) | ||
| area_b = (bx2 - bx1) * (by2 - by1) | ||
|
|
||
| # 2. Calculate the area of the overlap (A intersect B) | ||
|
|
||
| # Determine the coordinates of the overlap rectangle: (ix1, iy1) to (ix2, iy2) | ||
|
|
||
| # The left edge of the overlap is the max of the two left edges | ||
| ix1 = max(ax1, bx1) | ||
| # The right edge of the overlap is the min of the two right edges | ||
| ix2 = min(ax2, bx2) | ||
|
|
||
| # The bottom edge of the overlap is the max of the two bottom edges | ||
| iy1 = max(ay1, by1) | ||
| # The top edge of the overlap is the min of the two top edges | ||
| iy2 = min(ay2, by2) | ||
|
|
||
| # Calculate the width and height of the overlap | ||
| overlap_width = max(0, ix2 - ix1) | ||
| overlap_height = max(0, iy2 - iy1) | ||
|
|
||
| # The max(0, ...) ensures that if the rectangles do not overlap | ||
| # (e.g., ix2 < ix1), the width/height is 0, and the overlap_area is 0. | ||
| overlap_area = overlap_width * overlap_height | ||
|
|
||
| # 3. Apply the Inclusion-Exclusion Principle | ||
| total_area = area_a + area_b - overlap_area | ||
|
|
||
| return total_area |
There was a problem hiding this comment.
Add return type + docstring/doctest + reference URL (per repo checklist).
compute_area is missing a return type annotation and (per the PR checklist you referenced) a doctest + a URL comment pointing to Wikipedia/similar.
-def compute_area(
+def compute_area(
ax1: int, ay1: int, ax2: int, ay2: int, bx1: int, by1: int, bx2: int, by2: int
-):
+) -> int:
+ """
+ Compute total area covered by two axis-aligned rectangles.
+
+ Reference: https://en.wikipedia.org/wiki/Inclusion%E2%80%93exclusion_principle
+
+ >>> compute_area(0, 0, 2, 2, 1, 1, 3, 3)
+ 7
+ """(Optional hardening): consider guarding against reversed coordinates (e.g., ax2 < ax1) to avoid negative areas if callers violate constraints.
| You are given the coordinates of two axis-aligned rectangles in a 2D plane. Your task is to calculate the total area | ||
| covered by both rectangles. | ||
|
|
||
| The first rectangle is specified by the coordinates of its bottom-left corner (ax1, ay1) and top-right corner (ay1, ay2). | ||
|
|
||
| Similarly, the second rectangle is defined by its bottom-left corner (bx1, by1) and top-right corner (bx2, by2). |
There was a problem hiding this comment.
Fix coordinate typo in rectangle definition (ax2 vs ay1).
Line 6 currently says top-right is (ay1, ay2); that should be (ax2, ay2).
-The first rectangle is specified by the coordinates of its bottom-left corner (ax1, ay1) and top-right corner (ay1, ay2).
+The first rectangle is specified by the coordinates of its bottom-left corner (ax1, ay1) and top-right corner (ax2, ay2).Also applies to: 14-17
🤖 Prompt for AI Agents
In pymath/rectangle_area/README.md around lines 3-8 and again around lines
14-17, the rectangle coordinate descriptions misuse ax2 as ay1: change the
top-right coordinate of the first rectangle from (ay1, ay2) to (ax2, ay2) in
both occurrences so the first rectangle is defined by bottom-left (ax1, ay1) and
top-right (ax2, ay2).
BrianLusina
changed the title
BrianLusina
added
enhancement
Algorithm
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
puzzles/arrays/max_consecutive_ones/__init__.py (1)
44-45: Consider more Pythonic empty check.The empty check is correct but could be more idiomatic.
- if len(nums) == 0: + if not nums: return 0puzzles/arrays/max_consecutive_ones/test_max_consecutive_ones.py (1)
17-27: Consider adding edge case tests.The parameterized tests provide good coverage for typical scenarios. Consider adding a few edge cases to make the test suite more comprehensive:
@parameterized.expand([ ([1, 1, 1, 0, 0, 0, 1, 1, 1, 1, 0], 4), ([0, 0, 1, 1, 0, 0, 1, 1, 1, 0, 1, 1, 0, 0, 0, 1, 1, 1, 1], 4), ([1,1,0,0,1,1,1,0,0,1,0], 3), ([1,1,0,0,1,1], 2), ([1,1,1,0,1,1,1,1], 4), ([0,0,0,0], 0), + ([], 0), # empty array + ([1,1,1,1], 4), # all ones + ([0], 0), # single zero + ([1], 1), # single one ]) def test_find_max_consecutive_ones(self, nums: List[int], expected: int):
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
puzzles/arrays/max_consecutive_ones/README.md(1 hunks)puzzles/arrays/max_consecutive_ones/__init__.py(1 hunks)puzzles/arrays/max_consecutive_ones/test_max_consecutive_ones.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- puzzles/arrays/max_consecutive_ones/README.md
🧰 Additional context used
🧬 Code graph analysis (1)
puzzles/arrays/max_consecutive_ones/test_max_consecutive_ones.py (1)
puzzles/arrays/max_consecutive_ones/__init__.py (2)
longest_ones(4-22)find_max_consecutive_ones(24-55)
🔇 Additional comments (3)
puzzles/arrays/max_consecutive_ones/__init__.py (1)
46-55: LGTM!The algorithm implementation is correct and efficient. The logic properly maintains the current consecutive count and maximum count with a single pass through the array, achieving the documented O(n) time and O(1) space complexity.
puzzles/arrays/max_consecutive_ones/test_max_consecutive_ones.py (2)
2-4: LGTM!The import additions are appropriate for the parameterized test refactoring and type annotations.
9-15: LGTM!The parameterized test structure properly exercises the
longest_onesfunction with multiple scenarios, improving test coverage.
| def find_max_consecutive_ones(nums: List[int]) -> int: | ||
| """ | ||
| Finds the maximum consecutive ones in a binary array and returns it. | ||
| The most straightforward way to solve this is to use a single pass through the array, keeping track of two values | ||
| as we go. First, we need a counter for the current streak of consecutive 1s we're seeing. Second, we need to | ||
| remember the maximum streak we've encountered so far. | ||
| As we examine each element, if we see a 1, we increment our current streak counter. If we see a 0, that breaks our | ||
| streak, so we reset the counter to 0. Importantly, every time we update our current streak, we check whether it's | ||
| larger than our maximum and update the maximum if needed. | ||
| Time complexity is O(n) where n is the length of the input array | ||
| Space complexity is O(1) as no extra space is required | ||
| Args: | ||
| nums(list): a list of 1s and 0s. | ||
| Returns: | ||
| int: maximum number of consecutive 1s in the nums binary array | ||
| """ |
There was a problem hiding this comment.
Add reference URL as per PR checklist.
The PR checklist confirms "All new algorithms include a URL in comments pointing to Wikipedia or similar," but no reference URL is present for this algorithm. Consider adding a comment with a link to the problem description or relevant documentation.
For example:
+# Reference: https://leetcode.com/problems/max-consecutive-ones/
+# or https://en.wikipedia.org/wiki/Longest_run
def find_max_consecutive_ones(nums: List[int]) -> int:
"""
Finds the maximum consecutive ones in a binary array and returns it.📝 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 find_max_consecutive_ones(nums: List[int]) -> int: | |
| """ | |
| Finds the maximum consecutive ones in a binary array and returns it. | |
| The most straightforward way to solve this is to use a single pass through the array, keeping track of two values | |
| as we go. First, we need a counter for the current streak of consecutive 1s we're seeing. Second, we need to | |
| remember the maximum streak we've encountered so far. | |
| As we examine each element, if we see a 1, we increment our current streak counter. If we see a 0, that breaks our | |
| streak, so we reset the counter to 0. Importantly, every time we update our current streak, we check whether it's | |
| larger than our maximum and update the maximum if needed. | |
| Time complexity is O(n) where n is the length of the input array | |
| Space complexity is O(1) as no extra space is required | |
| Args: | |
| nums(list): a list of 1s and 0s. | |
| Returns: | |
| int: maximum number of consecutive 1s in the nums binary array | |
| """ | |
| # Reference: https://leetcode.com/problems/max-consecutive-ones/ | |
| # or https://en.wikipedia.org/wiki/Longest_run | |
| def find_max_consecutive_ones(nums: List[int]) -> int: | |
| """ | |
| Finds the maximum consecutive ones in a binary array and returns it. | |
| The most straightforward way to solve this is to use a single pass through the array, keeping track of two values | |
| as we go. First, we need a counter for the current streak of consecutive 1s we're seeing. Second, we need to | |
| remember the maximum streak we've encountered so far. | |
| As we examine each element, if we see a 1, we increment our current streak counter. If we see a 0, that breaks our | |
| streak, so we reset the counter to 0. Importantly, every time we update our current streak, we check whether it's | |
| larger than our maximum and update the maximum if needed. | |
| Time complexity is O(n) where n is the length of the input array | |
| Space complexity is O(1) as no extra space is required | |
| Args: | |
| nums(list): a list of 1s and 0s. | |
| Returns: | |
| int: maximum number of consecutive 1s in the nums binary array | |
| """ |
🤖 Prompt for AI Agents
puzzles/arrays/max_consecutive_ones/__init__.py lines 24-43: the function
docstring is missing the required reference URL per the PR checklist; add a
short comment or line in the docstring pointing to an authoritative source
(e.g., LeetCode problem page or Wikipedia) for the "Max Consecutive Ones"
problem, placing the URL near the top of the docstring or immediately above the
function definition so it’s clearly associated with the algorithm.
Describe your change:
Rectangle area
Checklist:
Fixes: #{$ISSUE_NO}.Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.