Add matrix_shortest_path_using_depth_first_search.py #7721
Conversation
finding the shortest path of a non symmetrical matrix (m x n) between given start and end points including consideration of obstacles using depth first search approach.
algorithms-keeper
bot
added
require descriptive names
![algorithms-keeper[bot]](https://avatars.githubusercontent.com/in/235834?s=60&v=4){kind=small}
There was a problem hiding this comment.
Click here to look at the relevant links ⬇️
🔗 Relevant Links
Repository:
Python:
Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.
algorithms-keeper commands and options
algorithms-keeper actions can be triggered by commenting on this PR:
@algorithms-keeper reviewto trigger the checks for only added pull request files@algorithms-keeper review-allto trigger the checks for all the pull request files, including the modified files. As we cannot post review comments on lines not part of the diff, this command will post all the messages in one comment.NOTE: Commands are in beta and so this feature is restricted only to a member or owner of the organization.
| current_shortest_path_distance = math.inf | ||
|
|
||
|
|
||
| def find_shortest_path_using_dfs(matrix: list, i: int, j: int, distance: int) -> None: |
There was a problem hiding this comment.
As there is no test file in this pull request nor any test function or class in the file path_tracing/matrix_shortest_path_using_depth_first_search.py, please provide doctest for the function find_shortest_path_using_dfs
Please provide descriptive name for the parameter: i
Please provide descriptive name for the parameter: j
Added descriptive names for i and j parameters of find_shortest_path_using_dfs function Added doctests for find_shortest_path_using_dfs function
algorithms-keeper
bot
added
tests are failing
| None | ||
| >>> find_shortest_path_using)dfs([[1], [-1, 's', 1], [-1, -1, 'e', -1]], 2, 1, 0) | ||
| """ | ||
| global current_shortest_path_distance |
There was a problem hiding this comment.
Using global is strongly recommended against. Perhaps return current_shortest_path_distance
| None | ||
| >>> find_shortest_path_using_dfs([[1, 1, 1], [1, 1,'s'], [1, -1, 'e']], 1, 2, 0) | ||
| None | ||
| >>> find_shortest_path_using)dfs([[1], [-1, 's', 1], [-1, -1, 'e', -1]], 2, 1, 0) |
There was a problem hiding this comment.
| >>> find_shortest_path_using)dfs([[1], [-1, 's', 1], [-1, -1, 'e', -1]], 2, 1, 0) | |
| >>> find_shortest_path_using_dfs([[1], [-1, 's', 1], [-1, -1, 'e', -1]], 2, 1, 0) |
| >>> find_shortest_path_using_dfs([['s', 1, 1], [1, -1, 1], [-1, 1, 'e']], 0, 0, 0) | ||
| None | ||
| >>> find_shortest_path_using_dfs([[-1, 1], ['s', 1], [-1, 1], [1, 'e']], 1, 0, 0) | ||
| None | ||
| >>> find_shortest_path_using_dfs([[1, 1, 1], [1, 1,'s'], [1, -1, 'e']], 1, 2, 0) | ||
| None | ||
| >>> find_shortest_path_using)dfs([[1], [-1, 's', 1], [-1, -1, 'e', -1]], 2, 1, 0) |
There was a problem hiding this comment.
There is no point of these doctests if they return None. All this doctest is really doing is checking for errors, which only needs to be done once.
|
|
||
|
|
||
| def find_shortest_path_using_dfs( | ||
| matrix: list, row: int, col: int, distance: int |
|
|
||
| if __name__ == "__main__": | ||
| shortest_path_distance = helper(matrix) | ||
|
|
| matrix = [ | ||
| [START_POS, VALID_POS, OBSTACLE_POS], | ||
| [VALID_POS, VALID_POS, VALID_POS], | ||
| [END_POS, OBSTACLE_POS, VALID_POS], | ||
| [OBSTACLE_POS, OBSTACLE_POS, OBSTACLE_POS], | ||
| ] | ||
|
|
||
| if __name__ == "__main__": |
There was a problem hiding this comment.
| matrix = [ | |
| [START_POS, VALID_POS, OBSTACLE_POS], | |
| [VALID_POS, VALID_POS, VALID_POS], | |
| [END_POS, OBSTACLE_POS, VALID_POS], | |
| [OBSTACLE_POS, OBSTACLE_POS, OBSTACLE_POS], | |
| ] | |
| if __name__ == "__main__": | |
| if __name__ == "__main__": | |
| matrix = [ | |
| [START_POS, VALID_POS, OBSTACLE_POS], | |
| [VALID_POS, VALID_POS, VALID_POS], | |
| [END_POS, OBSTACLE_POS, VALID_POS], | |
| [OBSTACLE_POS, OBSTACLE_POS, OBSTACLE_POS], | |
| ] | |
It is only being used inside the if, so that is where it will go
Modified find_shortest_path_using_dfs with proper doctest features
algorithms-keeper
bot
added
the
require tests
There was a problem hiding this comment.
Click here to look at the relevant links ⬇️
🔗 Relevant Links
Repository:
Python:
Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.
algorithms-keeper commands and options
algorithms-keeper actions can be triggered by commenting on this PR:
@algorithms-keeper reviewto trigger the checks for only added pull request files@algorithms-keeper review-allto trigger the checks for all the pull request files, including the modified files. As we cannot post review comments on lines not part of the diff, this command will post all the messages in one comment.NOTE: Commands are in beta and so this feature is restricted only to a member or owner of the organization.
| current_shortest_path_distance = math.inf | ||
|
|
||
|
|
||
| def find_shortest_path_using_dfs( |
There was a problem hiding this comment.
As there is no test file in this pull request nor any test function or class in the file path_tracing/matrix_shortest_path_using_depth_first_search.py, please provide doctest for the function find_shortest_path_using_dfs
algorithms-keeper
bot
removed
the
tests are failing
algorithms-keeper
bot
removed
the
require tests
There was a problem hiding this comment.
-
Like @CaedenPH said, you shouldn't be using global variables, but it would take an unnecessary amount of work to refactor this file to not use them.
-
Why create a new directory
path_tracing? We want to avoid creating new directories unless an algorithm truly doesn't fall under any of the existing directories. -
I take issue with the premise of this PR. DFS is an inherently really poor choice of algorithm here because DFS explores as far as possible before backtracking, meaning it needs to traverse every possible path to find the shortest one. If you wish to open a new PR to implement this using BFS instead, I think it'd be much more appropriate.
-
I kind of feel that this file is more a specific use case of an algorithm (namely DFS) rather than purely implementing an algorithm itself. In any case, I don't think this is the right algorithm for the job anyway.
Describe your change:
finding the shortest path of a non symmetrical matrix (m x n) between given start and end points including consideration of obstacles using depth first search approach.
Checklist:
Fixes: #{$ISSUE_NO}.