search

igraph / rigraph

igraph R package
https://r.igraph.org
532 stars 200 forks source link

chore: Update vendored sources to igraph/igraph@0f383457a13864b86a29eed4faf4efaade4c1254 #1299

Open krlmlr opened 4 months ago

krlmlr commented 4 months ago

Update to the develop branch of igraph/C. @Antonov548: Can you please pick up from here?

Closes #1296.

aviator-app[bot] commented 4 months ago

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 pull request is currently open (not queued).

How to merge

To merge this PR, comment /aviator merge or add the mergequeue label.

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.
aviator-app[bot] commented 4 months ago

This pull request failed to merge: this PR is in draft state. Remove the blocked label to re-queue.

Additional debug info: PR was marked as draft after queueing

aviator-app[bot] commented 4 months ago

This pull request can't be queued because it's currently a draft.

Antonov548 commented 3 months ago

@krlmlr I couldn't get from where is version of igraph/C which is used in this pull request and why there so much difference with the current. For me it seems it's some old version.

Antonov548 commented 3 months ago

Was this line added on purpose? https://github.com/igraph/rigraph/blob/6eeb6e4a575a8c0f718f0606cfc8805932b9e604/tools/update-cigraph.sh#L58

krlmlr commented 3 months ago

Have you seen #1296?

See https://github.com/igraph/rigraph/blame/6eeb6e4a575a8c0f718f0606cfc8805932b9e604/tools/update-cigraph.sh#L58 for where the line in question was added. True, this should have been a comment in the code.

Antonov548 commented 3 months ago

@szhorvat @ntamas This is what I added manually to the C/igraph. There is missmutch of definition and declaration:

https://github.com/igraph/rigraph/blob/90bb9e9492cd5d2997ce53633f16325359da02fd/src/vendor/cigraph/src/isomorphism/bliss.cc#L261-L264

ntamas commented 3 months ago

Thanks, fixed in igraph/igraph@44541b9ef

krlmlr commented 3 months ago

Thanks. So, now we can compile, but tests fail? Can we identify which, and then perhaps create a checklist?

Antonov548 commented 3 months ago

Thanks. So, now we can compile, but tests fail? Can we identify which, and then perhaps create a checklist?

Some tests are failing with stop of execution. Can you remind please how I can run without stop of execution? To see all failing tests.

krlmlr commented 3 months ago 
TESTTHAT_PARALLEL=false R -q -e 'testthat::test_local(reporter = "location")
Antonov548 commented 2 months ago

Tests are passing 👍 Examples are failing 👎

szhorvat commented 2 months ago

To make this work well, I suggest basing the upgrade on the changelog, not on the tests. Reading the changelog is an absolute must to be able to adapt not just functions, but also their documentation, appropriately. Expect some subtle changes that will be missed if you rely on tests only.

https://github.com/igraph/igraph/blob/develop/CHANGELOG.md

szhorvat commented 2 months ago

In the meantime some more breaking changes were made on the C side.