Don't copy an immutable digraph when adding zero vertices #346
Conversation
wilfwilson
added
minor
There was a problem hiding this comment.
I think the manual entry needs to be amended to note that the returned graph is not copied if 0 new nodes are added. Since I think this PR changes the behaviour, it's not inconceivable that there's code out there (or in here) that relies on the old behaviour.
wilfwilson
force-pushed
the
improve-opers
branch
from
8e55500
to
5041f15
Compare
|
I have updated the manual entry for |
wilfwilson
force-pushed
the
improve-opers
branch
from
5041f15
to
a081ee0
Compare
wilfwilson
changed the title
| {D, labels} -> MakeImmutable(DigraphAddVertices(DigraphMutableCopy(D), | ||
| labels))); | ||
| function(D, labels) | ||
| if IsEmpty(labels) then |
There was a problem hiding this comment.
I know you didn't name this in your PR, but why is the argument named labels?
There was a problem hiding this comment.
Because this method adds Length(labels) new vertices, where the ith vertex to be added is given the label labels[i].
wilfwilson
force-pushed
the
improve-opers
branch
from
b654260
to
e3b42a7
Compare
Codecov Report
@@ Coverage Diff @@
## stable-1.3 #346 +/- ##
===========================================
Coverage 95.25% 95.25%
===========================================
Files 49 49
Lines 12352 12359 +7
===========================================
+ Hits 11766 11773 +7
Misses 586 586
|
|
Can you please rebase onto stable-1.3, and then I'll merge @wilfwilson ? |
wilfwilson
force-pushed
the
improve-opers
branch
from
e3b42a7
to
6c86d96
Compare
This is similar to #338 and #340, for edges (although I made these changes over a year ago!).