Skip to content

feat: implement IGraphAlg generic dispatch and C wrappers#45

Open
mahmudsudo wants to merge 6 commits into
JuliaGraphs:masterfrom
mahmudsudo:pr-graphs-api
Open

feat: implement IGraphAlg generic dispatch and C wrappers#45
mahmudsudo wants to merge 6 commits into
JuliaGraphs:masterfrom
mahmudsudo:pr-graphs-api

Conversation

@mahmudsudo

@mahmudsudo mahmudsudo commented Mar 28, 2026

Copy link
Copy Markdown

This PR completes the integration of the igraph C library into the Graphs.jl ecosystem.

  • Generic Dispatch: Implements IGraphAlg for calling C-backed implementations.
  • C Wrappers: Extends support for centrality, components, modularity, and isomorphism.
  • Interface Validation: Uses GraphsInterfaceChecker.jl for AbstractGraph adherence.
  • Cleanup: Aligns naming conventions and JET testing style with Graphs.jl.

Note: This PR is stacked on #43 and #44

@codecov

codecov Bot commented Mar 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 37.50000% with 110 lines in your changes missing coverage. Please review.
✅ Project coverage is 4.86%. Comparing base (f5aad57) to head (ade7207).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
src/graph_api_extensions.jl 0.00% 68 Missing ⚠️
src/graph_api.jl 54.54% 40 Missing ⚠️
src/types.jl 90.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##           master     #45      +/-   ##
=========================================
- Coverage    5.28%   4.86%   -0.43%     
=========================================
  Files           8       8              
  Lines        4311    4459     +148     
=========================================
- Hits          228     217      -11     
- Misses       4083    4242     +159     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mahmudsudo

Copy link
Copy Markdown
Author

@Krastanov

@mahmudsudo

Copy link
Copy Markdown
Author

@Krastanov

@Krastanov Krastanov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for pushing on this @mahmudsudo ! We do not have many volunteer reviewers right now, so reminders on review are appreciated (and necessary, otherwise we just forget to do a review).

Could you help me a bit to understand how these things fit together, I see your PRs 43, 44, 45 and the Graphs.jl PR 506. Let's not worry about 506 for now. In 43 we discussed that the PR is rather large and dealing with multiple independent tasks so I asked to have it split so that it is easy to review it. Thank you for doing that, it does make things much easier for me to proceed with. However, the PR description above is empty, so I am not sure the overall goals you are pursuing -- could you enumerate the changes/reasons?

I also left a few minor comments in.

Comment thread src/graph_api_extensions.jl Outdated
Comment thread test/test_interfaces.jl
Comment thread test/test_jet.jl
@Krastanov

Copy link
Copy Markdown
Member

Also, something might be messed up with the way you did the split of #42 -- it seems #43, #44, and #45 have significant overlaps. Are they meant to be reviewed/merged in a particular order or was there a mistake during the split?

@mahmudsudo

Copy link
Copy Markdown
Author

Also, something might be messed up with the way you did the split of #42 -- it seems #43, #44, and #45 have significant overlaps. Are they meant to be reviewed/merged in a particular order or was there a mistake during the split?

i updated the pr description to state the pr order

@mahmudsudo

Copy link
Copy Markdown
Author

@Krastanov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants