Skip to content
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

Resolve Issue #413 (OutNeighbours performance) #419

Merged

Conversation

james-d-mitchell
Copy link
Member

This PR makes OutNeighbours a function rather than an attribute, to avoid the performance penalty of the return value being copied for mutable digraphs. This might not be desirable, but I can't think why it isn't at the moment, and the fact that the test all run after this change at least partially indicates that it might be desirable.

This resolves both #413 and #318.

@codecov
Copy link

codecov bot commented Feb 28, 2021

Codecov Report

Merging #419 (4b1213e) into stable-1.4 (c2e8ffb) will decrease coverage by 0.00%.
The diff coverage is 80.00%.

@@              Coverage Diff               @@
##           stable-1.4     #419      +/-   ##
==============================================
- Coverage       97.22%   97.21%   -0.01%     
==============================================
  Files              45       45              
  Lines           12315    12315              
==============================================
- Hits            11973    11972       -1     
- Misses            342      343       +1     
Impacted Files Coverage Δ
src/digraphs.c 84.39% <66.66%> (-0.07%) ⬇️
gap/attr.gd 100.00% <100.00%> (ø)
gap/attr.gi 100.00% <100.00%> (ø)

@james-d-mitchell james-d-mitchell added the performance A label for issues or PR related to performance label Feb 28, 2021
Copy link
Collaborator

@wilfwilson wilfwilson left a comment

Choose a reason for hiding this comment

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

I've not checked that this is actually faster but I'm happy to accept that it is! I also can't think of a downside.

@james-d-mitchell james-d-mitchell merged commit 8c649c6 into digraphs:stable-1.4 Mar 1, 2021
@james-d-mitchell james-d-mitchell deleted the fix-issue-413 branch March 1, 2021 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance A label for issues or PR related to performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants