Skip to content

Conversation

whitead
Copy link
Contributor

@whitead whitead commented Aug 20, 2025

@sidnarayanan really need you on this one - don't know the consequences of this change in default behavior.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Changes the default behavior of the traverse method in the graph operations module to avoid requiring a NetworkX import. The topological_order parameter default is changed from True to False.

  • Changed default value of topological_order parameter from True to False
  • Updated corresponding documentation comment to explain the rationale

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Collaborator

@sidnarayanan sidnarayanan left a comment

Choose a reason for hiding this comment

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

Approving, modulo one request

def traverse(
self,
topological_order: bool = True,
topological_order: bool = False,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm cool with changing the default, but can you pass topological_order=True here:

for node in self.traverse():
? That's the only place we currently require topological sorting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants