-
Notifications
You must be signed in to change notification settings - Fork 3.9k
[MRG] ENH: Add animations for search algorithms #1157
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
[MRG] ENH: Add animations for search algorithms #1157
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty nice work, some minor things.
I also did some cleanup in the |
notebook4e.py
Outdated
return {(x + i * dx, y + i * dy) for i in range(length)} | ||
|
||
|
||
class AnimateProblem(GridProblem): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't there a way to get just the AnimateProblem
function in notebook.py
? I think the rest do not fit here, except random_lines
and line
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need methods from the class GridProblem
(which in turn is a subclass of Problem
) and removing the class will result in the addition of the same methods in AnimateProblem
class. I know that it seems odd to repeat the same code, but we can get away with that if these functions and classes are just utilities for internal use. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I took a shot at it and removed almost all the duplicated code. Runs fine with a few warnings from matplotlib
animation API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh! The warning was because the solution initially is empty and matplotlib is not able to find the required arguments to draw... I think it's fine.
@antmarakis is this ready to merge? |
oh, I see why the tests fail! totally forgot to add support for python 3.5. Lemme do that real quick! |
Fixes #1155
Add more interactive visualization of search algorithms. I have added almost all the search algorithms but still have left bidirectional search, iterative deepening search, and lrta*.