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

igraph usage #139

Open
clbarnes opened this issue Jan 9, 2024 · 2 comments
Open

igraph usage #139

clbarnes opened this issue Jan 9, 2024 · 2 comments

Comments

@clbarnes
Copy link
Collaborator

clbarnes commented Jan 9, 2024

As igraph is now a non-optional dependency, would it be possible to remove a lot of the infrastructure involved in switching it on and off?

At present, we run a "no-igraph" CI job but it actually does use igraph, so it's not testing anything. Case in point: using scipy's sparse array for geodesic distance calculation is currently broken, but not tested in navis, so it just breaks in pymaid (which switches off igraph in a different way).

@schlegelp
Copy link
Collaborator

schlegelp commented Jan 9, 2024

I fixed the issue in geodesic_matrix with a5d4b72 but your point is still well taken.

We could probably just rip out the alternative networkx-based implementations that exist in some of the functions and nobody would notice. I'm trying to find a reason to keep them but the only thing I can come up with is a future regression in igraph (or an improvement in networkx) that would cause us to favour a networkx-based implementation over igraph. Probably not a strong argument against dropping the code that's currently effectively dead.

@schlegelp
Copy link
Collaborator

schlegelp commented Jan 9, 2024

Oh I guess you mentioned yourself that we could test that results from igraph- and networkx-based implementations have the same result?

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

No branches or pull requests

2 participants