Skip to content

UPDATED rat_in_maze.py #9148

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

Merged
merged 16 commits into from
Oct 4, 2023
Merged

Conversation

Muhammadummerr
Copy link
Contributor

@Muhammadummerr Muhammadummerr commented Sep 30, 2023

Describe your change:

Fixes #9066
I have made updates to the rat_in_maze.py file to enhance code clarity.
These updates include:

  1. Adding more descriptive comments to explain the logic and steps of the maze-solving algorithm.
  2. Introducing variables that allow for easy customization of the source and destination cells within the maze.
  3. Refactor path representation in solution:
  • Previously, in solution, paths were represented as 1s, but I have updated it to use 0s to align with the standard convention where 0s represent paths and 1s represent walls.
  1. Updated the return type to 'solution maze' if a path exists; otherwise, it returns 'None'.
  • Add an algorithm?
  • Fix a bug or typo in an existing algorithm?
  • Documentation change?

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new Python files are placed inside an existing directory.
  • All filenames are in all lowercase characters with no spaces or dashes.
  • All functions and variable names follow Python naming conventions.
  • All function parameters and return values are annotated with Python type hints.
  • All functions have doctests that pass the automated testing.
  • All new algorithms include at least one URL that points to Wikipedia or another similar explanation.
  • If this pull request resolves one or more open issues then the description above includes the issue number(s) with a closing keyword: "Fixes #ISSUE-NUMBER".

@algorithms-keeper algorithms-keeper bot added enhancement This PR modified some existing files awaiting reviews This PR is ready to be reviewed labels Sep 30, 2023
source_column: int,
destination_row: int,
destination_column: int,
) -> list[list[int]] | None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning two different datatypes pushes work on the caller. Please consider raising an exception if there is no solution.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my remark in the general discussion of this Pull Request.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
) -> list[list[int]] | None:
) -> list[list[int]]:

@Muhammadummerr Muhammadummerr requested a review from cclauss October 1, 2023 12:35
@Muhammadummerr
Copy link
Contributor Author

@cclauss Is there something I'm missing?

@dlesnoff
Copy link

dlesnoff commented Oct 3, 2023

Ok so @Muhammadummerr old pull request was reopened here.

@cclauss Is there something I'm missing?

Returning two different datatypes pushes work on the caller. Please consider raising an exception if there is no solution.

@cclaus If an exception is raised, the caller still has to wrap each function call in a try-except clause and catch the correct exception. In Nim, I would use an Option type (the equivalent of Result type in Rust, I guess?).
Maybe in Python, the closest to an Option type would be a dataclass with a list[list[int]] attribute and a boolean flag?
The caller would have to check for the boolean flag before accessing the maze, but at least the program would not crash in that case.
Exceptions are frequent in other Python Algorithm snippets, but they are often used for a bad user input. Here, the None type is a frequent and expected output of the algorithm.

@cclauss
Copy link
Member

cclauss commented Oct 3, 2023

It is very true that Nim and Rust have cooler ways of dealing with exception states that Python does but raising Exceptions are quite Pythonic and are preferred to multiple different datatypes.

@Muhammadummerr
Copy link
Contributor Author

Okay,Based on my understanding of your request, the program should return both the solution maze and a flag.
1.If the flag's value is one, it will indicate that a solution exists, and the path in the solution maze will be represented by 0s.
2.If the flag is 0, it means that a solution was not found, and the solution maze will consist of only 1s.

@cclauss
Copy link
Member

cclauss commented Oct 3, 2023

No. The function should return a solution if there is one and raise an ValueError if there is no solution.

@Muhammadummerr
Copy link
Contributor Author

I have made these changes.

@Muhammadummerr
Copy link
Contributor Author

Okay, my mistake. I need to make some more changes. Thanks.

@dlesnoff
Copy link

dlesnoff commented Oct 4, 2023

Do we really care about changing the initial and destination cells in the maze?

I do not really see the benefit of it and it just makes the implementation harder to read and understand.

[0, 0, 0, 0, 1]
True
>>> solve_maze(maze,0,0,len(maze)-1,len(maze)-1)
[[0, 1, 1, 1, 1], [0, 0, 0, 0, 1], [1, 1, 1, 0, 1],\
Copy link

@dlesnoff dlesnoff Oct 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the output on the same line now? It makes reading the solution harder.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://docs.python.org/3/library/doctest.html#doctest.NORMALIZE_WHITESPACE will allow you to have doctests that ignore the whitespace so the matrix can look like a matrix.

@algorithms-keeper algorithms-keeper bot added the tests are failing Do not merge until tests pass label Oct 4, 2023
@Muhammadummerr
Copy link
Contributor Author

All Test case are passed on my device but failing in build check.

@cclauss
Copy link
Member

cclauss commented Oct 4, 2023

Looking better!

@cclauss
Copy link
Member

cclauss commented Oct 4, 2023

________________ [doctest] backtracking.rat_in_maze.solve_maze _________________
028 aiming to reach a destination cell.
029 The maze consists of walls (1s) and open paths (0s).
030 By providing custom row and column values, the source and destination
031 cells can be adjusted.
032 >>> maze = [[0, 1, 0, 1, 1],
033 ... [0, 0, 0, 0, 0],
034 ... [1, 0, 1, 0, 1],
035 ... [0, 0, 1, 0, 0],
036 ... [1, 0, 0, 1, 0]]
037 >>> solve_maze(maze,0,0,len(maze)-1,len(maze)-1)
Expected:
[[0, 1, 1, 1, 1],
[0, 0, 0, 0, 1],
[1, 1, 1, 0, 1],
[1, 1, 1, 0, 0],
[1, 1, 1, 1, 0]]
Got:
[[0, 1, 1, 1, 1], [0, 0, 0, 0, 1], [1, 1, 1, 0, 1], [1, 1, 1, 0, 0], [1, 1, 1, 1, 0]]

/home/runner/work/Python/Python/backtracking/rat_in_maze.py:37: DocTestFailure

@Muhammadummerr
Copy link
Contributor Author

________________ [doctest] backtracking.rat_in_maze.solve_maze _________________ 028 aiming to reach a destination cell. 029 The maze consists of walls (1s) and open paths (0s). 030 By providing custom row and column values, the source and destination 031 cells can be adjusted. 032 >>> maze = [[0, 1, 0, 1, 1], 033 ... [0, 0, 0, 0, 0], 034 ... [1, 0, 1, 0, 1], 035 ... [0, 0, 1, 0, 0], 036 ... [1, 0, 0, 1, 0]] 037 >>> solve_maze(maze,0,0,len(maze)-1,len(maze)-1) Expected: [[0, 1, 1, 1, 1], [0, 0, 0, 0, 1], [1, 1, 1, 0, 1], [1, 1, 1, 0, 0], [1, 1, 1, 1, 0]] Got: [[0, 1, 1, 1, 1], [0, 0, 0, 0, 1], [1, 1, 1, 0, 1], [1, 1, 1, 0, 0], [1, 1, 1, 1, 0]]

/home/runner/work/Python/Python/backtracking/rat_in_maze.py:37: DocTestFailure

This same test case passed on my local device because I used (optionflags=doctest.NORMALIZE_WHITESPACE), so why is it failing here?

@Muhammadummerr
Copy link
Contributor Author

Do we really care about changing the initial and destination cells in the maze?

I do not really see the benefit of it and it just makes the implementation harder to read and understand.

I guess; it would be more helpful for understanding to run the program with different source and destination points to find the path.

@algorithms-keeper algorithms-keeper bot removed the tests are failing Do not merge until tests pass label Oct 4, 2023
@Muhammadummerr
Copy link
Contributor Author

Does this program require any further changes, or is it okay as it is?

return False
return False


if __name__ == "__main__":
import doctest

doctest.testmod()
doctest.testmod(optionflags=doctest.NORMALIZE_WHITESPACE)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work?!?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work?!?

Yes, it's working on my local machine, but it's not working on the build test. That's why I added '# doctest: +NORMALIZE_WHITESPACE' to each test.

@algorithms-keeper algorithms-keeper bot removed the awaiting reviews This PR is ready to be reviewed label Oct 4, 2023
@algorithms-keeper algorithms-keeper bot added the awaiting reviews This PR is ready to be reviewed label Oct 4, 2023
@cclauss cclauss merged commit c16d2f8 into TheAlgorithms:master Oct 4, 2023
@algorithms-keeper algorithms-keeper bot removed the awaiting reviews This PR is ready to be reviewed label Oct 4, 2023
sedatguzelsemme pushed a commit to sedatguzelsemme/Python that referenced this pull request Sep 15, 2024
* UPDATED rat_in_maze.py

* Update reddit.py in Webprogramming b/c it was causing error in pre-commit tests while raising PR.

* UPDATED rat_in_maze.py

* fixed return type to only maze,otherwise raise valueError.

* fixed whitespaces error,improved matrix visual.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* updated.

* Try

* updated

* updated

* Apply suggestions from code review

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Christian Clauss <[email protected]>
@isidroas isidroas mentioned this pull request Jan 25, 2025
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This PR modified some existing files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Backtracking/Rat Maze
3 participants