Contributes to #9943: Add docstrings and doctests, fix is_safe logic, and improve code formatting - #12691
Open
akiels wants to merge 3 commits into
Open
Contributes to #9943: Add docstrings and doctests, fix is_safe logic, and improve code formatting #12691akiels wants to merge 3 commits into
is_safe logic, and improve code formatting #12691akiels wants to merge 3 commits into
Conversation
- Added descriptive docstrings for Matrix, is_safe, count_islands, and diffs methods - Added doctests covering both normal and edge cases for all methods - Improves didactic quality and test coverage
- Formatting code with pre-commit hooks (black, ruff) - Fixing typos - No functional changes
Previously, is_safe() could return the value of a cell (int 1 or 0), which caused test failures. This fix ensures that is_safe() returns True if the cell is safe to visit and False otherwise
mindaugl
approved these changes
May 14, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contributes to #9943
This PR improves the
count-islands-in-matriximplementation by:Adding docstrings and doctests for all methods (
Matrix,is_safe,diffs, andcount_islands)Applying code formatting for consistency and pre-commit compliance (e.g., removing trailing whitespace, fixing parentheses).
Fixing a logic bug in
is_safe:Previously,
is_safe()could returnintvalues 1 or 0 instead of expectedbooleanvalues (TrueorFalse).This was corrected by adding a condition to
self.graph[i][j], which ensures thatis_safe()doesn't return the value of a cell (intvalues 1 or 0) but instead returnsTrueif the cell has value 1 andFalseotherwise (value 0).Since this is a small logic fix, I include it in this PR along with the docs, tests, and formatting changes.
All changes pass local testing with
doctestand pre-commit hooks.These changes together improve the clarity, functionality, and test coverage of the implementation.
Thank you for reviewing!