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

Unpin igraph version #4

Merged
merged 3 commits into from
Sep 15, 2023
Merged

Unpin igraph version #4

merged 3 commits into from
Sep 15, 2023

Conversation

johandahlberg
Copy link
Contributor

Description

This PR allows the igraph dependency to vary in the patch version. This will make it easier to build working conda recipes.

To make this work we have to explicitly add the vertices when creating the graph. This seems (as far as I've been able to tell) to be an undocumented change in how the igraph APIs work.

In addition unpinning the igraph dependency I've removed some unused data that caused warnings in the tests.

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

This has been tested with the unit test suite.

@johandahlberg johandahlberg merged commit 4384fde into dev Sep 15, 2023
13 checks passed
@fbdtemme fbdtemme deleted the feature/update-igraph branch September 15, 2023 14:43
@szhorvat
Copy link

This seems (as far as I've been able to tell) to be an undocumented change in how the igraph APIs work.

Can you clarify what you are referring to?

Are you looking for the use_vids=False parameter of Graph.DataFrame?

https://python.igraph.org/en/stable/api/igraph.Graph.html#DataFrame

The default changed to True in python-igraph 0.10.0, see https://github.com/igraph/python-igraph/blob/main/CHANGELOG.md#0100---2022-09-05

@johandahlberg
Copy link
Contributor Author

@szhorvat We've been usinguse_vids=False previously, so I don't think it's that: https://github.com/PixelgenTechnologies/pixelator/pull/4/files#diff-98ab30b9ad4a777bbe400bfd140d50981a41d13c8ef6869ceac155a24b681716L65 The only way that I could get our test suite to pass was to add vertices=.... It's quite possible I'm missing something here, though.

Thank you for pitching in! If you have an other theories about what's going on here I'd be happy to hear them. And a big welcome as the first external contributor in Pixelator community!

@szhorvat
Copy link

When vertices are not provided, igraph does basically the same thing as what you did in this PR, see

https://github.com/igraph/python-igraph/blob/main/src/igraph/io/objects.py#L462

If you notice any difference (other than vertex ordering) between two graphs constructed with and without providing such a vertices parameter, please report it to igraph.

@johandahlberg
Copy link
Contributor Author

Thank you! I will look in to it and see if I can construct a minimal example that replicates the behavior.

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.

3 participants