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

fix: fix includig diagonal elements in dense adjacency matrices #1437

Merged
merged 2 commits into from
Aug 29, 2024

Conversation

szhorvat
Copy link
Member

@szhorvat szhorvat commented Jul 19, 2024

Fixes #1429

This does fix the issue reported in #1429, and is ready to go right now in that sense. But you may want to polish it up to prepare the ground for future improvements. I leave it in your hands @Antonov548 @krlmlr

You may also want to add some tests (weighted and unweighted undirected case).

Copy link
Contributor

aviator-app bot commented Jul 19, 2024

Current Aviator status

Aviator will automatically update this comment as the status of the PR changes.
Comment /aviator refresh to force Aviator to re-examine your PR (or learn about other /aviator commands).

This PR was merged using Aviator.


See the real-time status of this PR on the Aviator webapp.
Use the Aviator Chrome Extension to see the status of your PR within GitHub.

@krlmlr
Copy link
Contributor

krlmlr commented Aug 28, 2024

@maelle: Can you please add a test to this PR? Looks good to me otherwise.

@maelle
Copy link
Contributor

maelle commented Aug 29, 2024

On it now 😸

@maelle
Copy link
Contributor

maelle commented Aug 29, 2024

All test cases except the fourth one fail for the main branch. 🎉

Copy link
Contributor

@krlmlr krlmlr left a comment

Choose a reason for hiding this comment

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

Nice, thanks!

Copy link
Contributor

aviator-app bot commented Aug 29, 2024

This pull request failed to merge: PR cannot be automatically rebased, please rebase manually to continue. Once the issues are resolved, remove the blocked label and re-queue the pull request. Note that the pull request will be automatically re-queued if it has the mergequeue label.

Additional debug info: Failed to rebase this PR onto the latest changes from the base branch. You will probably need to rebase this PR manually and resolve conflicts).

@krlmlr krlmlr removed the blocked label Aug 29, 2024
@aviator-app aviator-app bot merged commit f09728c into main Aug 29, 2024
20 checks passed
@aviator-app aviator-app bot deleted the fix/get_adjacency_loops branch August 29, 2024 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

no diag element returned by "as_adjacency_matrix" when sparse = F
3 participants