Skip to content

Contributes to #9943: Add docstrings and doctests, fix is_safe logic, and improve code formatting #12691

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

akiels
Copy link

@akiels akiels commented Apr 28, 2025

Contributes to #9943

This PR improves the count-islands-in-matrix implementation by:

  1. Adding docstrings and doctests for all methods (Matrix, is_safe, diffs, and count_islands)

  2. Applying code formatting for consistency and pre-commit compliance (e.g., removing trailing whitespace, fixing parentheses).

  3. Fixing a logic bug in is_safe:

  • Previously, is_safe() could return int values 1 or 0 instead of expected boolean values (True or False).

  • This was corrected by adding a condition to self.graph[i][j], which ensures that is_safe() doesn't return the value of a cell (int values 1 or 0) but instead returns True if the cell has value 1 and False otherwise (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 doctest and pre-commit hooks.

These changes together improve the clarity, functionality, and test coverage of the implementation.

Thank you for reviewing!

akiels added 3 commits April 28, 2025 18:36
- 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
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants