-
Notifications
You must be signed in to change notification settings - Fork 86
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
Update networkx version #4035
Update networkx version #4035
Conversation
Codecov Report
@@ Coverage Diff @@
## main #4035 +/- ##
========================================
- Coverage 99.7% 84.4% -15.3%
========================================
Files 349 349
Lines 37242 37257 +15
========================================
- Hits 37121 31425 -5696
- Misses 121 5832 +5711
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
@@ -271,15 +263,11 @@ def test_instantiate_with_parameters(example_graph): | |||
} | |||
component_graph.instantiate(parameters) | |||
|
|||
expected_order = [ |
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.
is the component ordering changing? Just wondering why we're changing these tests
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.
Yep, that was the change that caused us to pin the version a year and a half ago. For non-linear graphs, the order is now more breadth-first rather than depth-first. It's still a correct ordering, which is why the tests now simply assert that components that must be fit/transformed before others still are.
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.
Thanks!
Closes #3972
Networkx changed the topological order of their DAGs with version 2.6, which broke some of our tests that validate the computation order. We pinned it and left it there. Changes here unpin the version and update the tests to be more flexible about the order of computations, so long as the data flow is still maintained.