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
Fix custom weight attribute for Mehlhorn #6681
Conversation
Using a custom weight attribute in the Mehlhorn implementation gave a KeyError. Reason: Edges in G_1_prime are given "weight" attributes, but upon updating, the weight (=/= "weight") of the existing edge in G_1_prime is get. Fix: set and get only "weight" attributes for G_1_prime.
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 @kalkoen! Could you also add a test for this in the tests file https://github.com/networkx/networkx/blob/main/networkx/algorithms/approximation/tests/test_steinertree.py?
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.
I took the liberty of pushing up a couple tests for cases where the edge attribute name is something other than the default "weight". The first test covers the bugfix (i.e. fails on current main
) - the second covers a similar scenario, but with multigraphs.
This LGTM but should get another pair of eyes. Thanks @kalkoen !
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.
* Fix custom weight attribute for Mehlhorn Using a custom weight attribute in the Mehlhorn implementation gave a KeyError. Reason: Edges in G_1_prime are given "weight" attributes, but upon updating, the weight (=/= "weight") of the existing edge in G_1_prime is get. Fix: set and get only "weight" attributes for G_1_prime. * Add test for non-default edge attr weight. * TST: parametrize on steiner method. * Add test for multigraph with non-default edge attr. --------- Co-authored-by: Ross Barnowski <rossbar@berkeley.edu>
Using a custom weight attribute in the Mehlhorn implementation gave a KeyError. Reason: Edges in G_1_prime are given "weight" attributes, but upon updating, the weight (=/= "weight") of the existing edge in G_1_prime is get. Fix: set and get only "weight" attributes for G_1_prime.