Depth First Search of graph in Python

The name of the pictureThe name of the pictureThe name of the pictureClash Royale CLAN TAG#URR8PPP





.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;







up vote
0
down vote

favorite












Task: Do DFS traversal from a given given graph and print output.



Is this code the correct implementation of DFS traversal in Graph?



from collections import defaultdict

class Graph(object):
def __init__(self):
self.graph = defaultdict(list)

def add_edge(self, u, v):
self.graph[u].append(v)

def depth_first_search(self,node):
visited =
stack = [node]

while stack:

node = stack.pop()
if node not in visited:
print node
visited.append(node)

for i in self.graph[node]:
stack.append(i)



G = Graph()
G.add_edge(1,2)
G.add_edge(1,4)
G.add_edge(1,5)
G.add_edge(2,3)
G.add_edge(3,5)
G.depth_first_search(1)






share|improve this question



















  • What is your expected behavior for cyclic graphs? You only have an acyclic graph in your example and DFS struggles with cycles on an arbitrary graph.
    – FirefoxMetzger
    Apr 16 at 4:35






  • 1




    looks good, the only thing to improve is that you could define what node is and based on that you can add hashing algorithm for it. so then instead of having visited as list you could have it as a set. Which will improve your in lookup from O(N) to O(1)
    – Alex
    Apr 16 at 13:51
















up vote
0
down vote

favorite












Task: Do DFS traversal from a given given graph and print output.



Is this code the correct implementation of DFS traversal in Graph?



from collections import defaultdict

class Graph(object):
def __init__(self):
self.graph = defaultdict(list)

def add_edge(self, u, v):
self.graph[u].append(v)

def depth_first_search(self,node):
visited =
stack = [node]

while stack:

node = stack.pop()
if node not in visited:
print node
visited.append(node)

for i in self.graph[node]:
stack.append(i)



G = Graph()
G.add_edge(1,2)
G.add_edge(1,4)
G.add_edge(1,5)
G.add_edge(2,3)
G.add_edge(3,5)
G.depth_first_search(1)






share|improve this question



















  • What is your expected behavior for cyclic graphs? You only have an acyclic graph in your example and DFS struggles with cycles on an arbitrary graph.
    – FirefoxMetzger
    Apr 16 at 4:35






  • 1




    looks good, the only thing to improve is that you could define what node is and based on that you can add hashing algorithm for it. so then instead of having visited as list you could have it as a set. Which will improve your in lookup from O(N) to O(1)
    – Alex
    Apr 16 at 13:51












up vote
0
down vote

favorite









up vote
0
down vote

favorite











Task: Do DFS traversal from a given given graph and print output.



Is this code the correct implementation of DFS traversal in Graph?



from collections import defaultdict

class Graph(object):
def __init__(self):
self.graph = defaultdict(list)

def add_edge(self, u, v):
self.graph[u].append(v)

def depth_first_search(self,node):
visited =
stack = [node]

while stack:

node = stack.pop()
if node not in visited:
print node
visited.append(node)

for i in self.graph[node]:
stack.append(i)



G = Graph()
G.add_edge(1,2)
G.add_edge(1,4)
G.add_edge(1,5)
G.add_edge(2,3)
G.add_edge(3,5)
G.depth_first_search(1)






share|improve this question











Task: Do DFS traversal from a given given graph and print output.



Is this code the correct implementation of DFS traversal in Graph?



from collections import defaultdict

class Graph(object):
def __init__(self):
self.graph = defaultdict(list)

def add_edge(self, u, v):
self.graph[u].append(v)

def depth_first_search(self,node):
visited =
stack = [node]

while stack:

node = stack.pop()
if node not in visited:
print node
visited.append(node)

for i in self.graph[node]:
stack.append(i)



G = Graph()
G.add_edge(1,2)
G.add_edge(1,4)
G.add_edge(1,5)
G.add_edge(2,3)
G.add_edge(3,5)
G.depth_first_search(1)








share|improve this question










share|improve this question




share|improve this question









asked Apr 15 at 22:44









Latika Agarwal

861216




861216











  • What is your expected behavior for cyclic graphs? You only have an acyclic graph in your example and DFS struggles with cycles on an arbitrary graph.
    – FirefoxMetzger
    Apr 16 at 4:35






  • 1




    looks good, the only thing to improve is that you could define what node is and based on that you can add hashing algorithm for it. so then instead of having visited as list you could have it as a set. Which will improve your in lookup from O(N) to O(1)
    – Alex
    Apr 16 at 13:51
















  • What is your expected behavior for cyclic graphs? You only have an acyclic graph in your example and DFS struggles with cycles on an arbitrary graph.
    – FirefoxMetzger
    Apr 16 at 4:35






  • 1




    looks good, the only thing to improve is that you could define what node is and based on that you can add hashing algorithm for it. so then instead of having visited as list you could have it as a set. Which will improve your in lookup from O(N) to O(1)
    – Alex
    Apr 16 at 13:51















What is your expected behavior for cyclic graphs? You only have an acyclic graph in your example and DFS struggles with cycles on an arbitrary graph.
– FirefoxMetzger
Apr 16 at 4:35




What is your expected behavior for cyclic graphs? You only have an acyclic graph in your example and DFS struggles with cycles on an arbitrary graph.
– FirefoxMetzger
Apr 16 at 4:35




1




1




looks good, the only thing to improve is that you could define what node is and based on that you can add hashing algorithm for it. so then instead of having visited as list you could have it as a set. Which will improve your in lookup from O(N) to O(1)
– Alex
Apr 16 at 13:51




looks good, the only thing to improve is that you could define what node is and based on that you can add hashing algorithm for it. so then instead of having visited as list you could have it as a set. Which will improve your in lookup from O(N) to O(1)
– Alex
Apr 16 at 13:51










1 Answer
1






active

oldest

votes

















up vote
1
down vote



accepted










Classic DFS doesn't use any pruning. That means you should not have a list of visited nodes; Hence, my question in the comments. This means that for cyclic graphs DFS does not terminate by default which is why people commonly do pruning on visited nodes; however, revisiting nodes may be wanted, e.g. "list all paths from edge 1 to edge 5". Making the choice of using visited not only makes your graph acyclic, but rather "tree-ifys" (technical term ;-) ) it.




Assuming visited is an okay thing to do you don't want to check if node not in visited: after poping an element but rather before you insert it. It saves you the overhead of appending and popping visited nodes which can be quite substantial.



Further, visited should be a set (as stated in @Alex 's comment).



It could also be a good idea to use an actual queue object, be that collections.deque (for FIFO / LIFO) or a queue.PriorityQueue. The latter will slightly reduce performance (insert from O(1) to O(log(queue_size))), but offers a lot of added flexibility and easy scalability to graph search.



Setting the priority to: 1/depth is DFS, depth is BFS, sum(depth) (or sum(transition costs)) is Dijkstra's search, expected_cost_to_goal is greedy search, sum(transition costs) + expected_cost_to_goal is A*. I think that's a really cool property.






share|improve this answer























    Your Answer




    StackExchange.ifUsing("editor", function ()
    return StackExchange.using("mathjaxEditing", function ()
    StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
    StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
    );
    );
    , "mathjax-editing");

    StackExchange.ifUsing("editor", function ()
    StackExchange.using("externalEditor", function ()
    StackExchange.using("snippets", function ()
    StackExchange.snippets.init();
    );
    );
    , "code-snippets");

    StackExchange.ready(function()
    var channelOptions =
    tags: "".split(" "),
    id: "196"
    ;
    initTagRenderer("".split(" "), "".split(" "), channelOptions);

    StackExchange.using("externalEditor", function()
    // Have to fire editor after snippets, if snippets enabled
    if (StackExchange.settings.snippets.snippetsEnabled)
    StackExchange.using("snippets", function()
    createEditor();
    );

    else
    createEditor();

    );

    function createEditor()
    StackExchange.prepareEditor(
    heartbeatType: 'answer',
    convertImagesToLinks: false,
    noModals: false,
    showLowRepImageUploadWarning: true,
    reputationToPostImages: null,
    bindNavPrevention: true,
    postfix: "",
    onDemand: true,
    discardSelector: ".discard-answer"
    ,immediatelyShowMarkdownHelp:true
    );



    );








     

    draft saved


    draft discarded


















    StackExchange.ready(
    function ()
    StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f192147%2fdepth-first-search-of-graph-in-python%23new-answer', 'question_page');

    );

    Post as a guest






























    1 Answer
    1






    active

    oldest

    votes








    1 Answer
    1






    active

    oldest

    votes









    active

    oldest

    votes






    active

    oldest

    votes








    up vote
    1
    down vote



    accepted










    Classic DFS doesn't use any pruning. That means you should not have a list of visited nodes; Hence, my question in the comments. This means that for cyclic graphs DFS does not terminate by default which is why people commonly do pruning on visited nodes; however, revisiting nodes may be wanted, e.g. "list all paths from edge 1 to edge 5". Making the choice of using visited not only makes your graph acyclic, but rather "tree-ifys" (technical term ;-) ) it.




    Assuming visited is an okay thing to do you don't want to check if node not in visited: after poping an element but rather before you insert it. It saves you the overhead of appending and popping visited nodes which can be quite substantial.



    Further, visited should be a set (as stated in @Alex 's comment).



    It could also be a good idea to use an actual queue object, be that collections.deque (for FIFO / LIFO) or a queue.PriorityQueue. The latter will slightly reduce performance (insert from O(1) to O(log(queue_size))), but offers a lot of added flexibility and easy scalability to graph search.



    Setting the priority to: 1/depth is DFS, depth is BFS, sum(depth) (or sum(transition costs)) is Dijkstra's search, expected_cost_to_goal is greedy search, sum(transition costs) + expected_cost_to_goal is A*. I think that's a really cool property.






    share|improve this answer



























      up vote
      1
      down vote



      accepted










      Classic DFS doesn't use any pruning. That means you should not have a list of visited nodes; Hence, my question in the comments. This means that for cyclic graphs DFS does not terminate by default which is why people commonly do pruning on visited nodes; however, revisiting nodes may be wanted, e.g. "list all paths from edge 1 to edge 5". Making the choice of using visited not only makes your graph acyclic, but rather "tree-ifys" (technical term ;-) ) it.




      Assuming visited is an okay thing to do you don't want to check if node not in visited: after poping an element but rather before you insert it. It saves you the overhead of appending and popping visited nodes which can be quite substantial.



      Further, visited should be a set (as stated in @Alex 's comment).



      It could also be a good idea to use an actual queue object, be that collections.deque (for FIFO / LIFO) or a queue.PriorityQueue. The latter will slightly reduce performance (insert from O(1) to O(log(queue_size))), but offers a lot of added flexibility and easy scalability to graph search.



      Setting the priority to: 1/depth is DFS, depth is BFS, sum(depth) (or sum(transition costs)) is Dijkstra's search, expected_cost_to_goal is greedy search, sum(transition costs) + expected_cost_to_goal is A*. I think that's a really cool property.






      share|improve this answer

























        up vote
        1
        down vote



        accepted







        up vote
        1
        down vote



        accepted






        Classic DFS doesn't use any pruning. That means you should not have a list of visited nodes; Hence, my question in the comments. This means that for cyclic graphs DFS does not terminate by default which is why people commonly do pruning on visited nodes; however, revisiting nodes may be wanted, e.g. "list all paths from edge 1 to edge 5". Making the choice of using visited not only makes your graph acyclic, but rather "tree-ifys" (technical term ;-) ) it.




        Assuming visited is an okay thing to do you don't want to check if node not in visited: after poping an element but rather before you insert it. It saves you the overhead of appending and popping visited nodes which can be quite substantial.



        Further, visited should be a set (as stated in @Alex 's comment).



        It could also be a good idea to use an actual queue object, be that collections.deque (for FIFO / LIFO) or a queue.PriorityQueue. The latter will slightly reduce performance (insert from O(1) to O(log(queue_size))), but offers a lot of added flexibility and easy scalability to graph search.



        Setting the priority to: 1/depth is DFS, depth is BFS, sum(depth) (or sum(transition costs)) is Dijkstra's search, expected_cost_to_goal is greedy search, sum(transition costs) + expected_cost_to_goal is A*. I think that's a really cool property.






        share|improve this answer















        Classic DFS doesn't use any pruning. That means you should not have a list of visited nodes; Hence, my question in the comments. This means that for cyclic graphs DFS does not terminate by default which is why people commonly do pruning on visited nodes; however, revisiting nodes may be wanted, e.g. "list all paths from edge 1 to edge 5". Making the choice of using visited not only makes your graph acyclic, but rather "tree-ifys" (technical term ;-) ) it.




        Assuming visited is an okay thing to do you don't want to check if node not in visited: after poping an element but rather before you insert it. It saves you the overhead of appending and popping visited nodes which can be quite substantial.



        Further, visited should be a set (as stated in @Alex 's comment).



        It could also be a good idea to use an actual queue object, be that collections.deque (for FIFO / LIFO) or a queue.PriorityQueue. The latter will slightly reduce performance (insert from O(1) to O(log(queue_size))), but offers a lot of added flexibility and easy scalability to graph search.



        Setting the priority to: 1/depth is DFS, depth is BFS, sum(depth) (or sum(transition costs)) is Dijkstra's search, expected_cost_to_goal is greedy search, sum(transition costs) + expected_cost_to_goal is A*. I think that's a really cool property.







        share|improve this answer















        share|improve this answer



        share|improve this answer








        edited Apr 18 at 4:54


























        answered Apr 16 at 15:46









        FirefoxMetzger

        74628




        74628






















             

            draft saved


            draft discarded


























             


            draft saved


            draft discarded














            StackExchange.ready(
            function ()
            StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f192147%2fdepth-first-search-of-graph-in-python%23new-answer', 'question_page');

            );

            Post as a guest













































































            Popular posts from this blog

            Greedy Best First Search implementation in Rust

            Function to Return a JSON Like Objects Using VBA Collections and Arrays

            C++11 CLH Lock Implementation