feat: improve formatting of error messages from C by maelle · Pull Request #2515 · igraph/rigraph

maelle · 2026-01-06T11:47:40Z

One thing we might consider adding, is a period at the end of the message if it's not already there.

szhorvat · 2026-01-06T13:48:13Z

One thing we might consider adding, is a period at the end of the message if it's not already there.

I've added punctuation at the end of most messages in the C core now, but a few are still left out. This is a nice-to-have aesthetic feature, it's not an absolute must unless you want to append extra text at the end of the message. We do correct for missing punctuation in both the Python and Mathematica interfaces.

src/rinterface_extra.c

szhorvat · 2026-01-06T13:56:10Z

One thing we might consider adding, is a period at the end of the message if it's not already there.

Isn't this already there with maybe_add_puctuation?

LGTM, but I didn't test.

Co-authored-by: Szabolcs Horvát <szhorvat@gmail.com>

maelle · 2026-01-13T08:46:05Z

Thank you @szhorvat @schochastics!

Run: https://github.com/igraph/rigraph/actions/runs/20950334632

maelle · 2026-01-13T09:22:08Z

@szhorvat with your change the path wasn't simplified any longer, does my fix make any sense?

szhorvat · 2026-01-13T10:47:59Z

I'm sorry, I can't seem to see your fix, but I'm in a rush and may be missing something. I believe my mistake is that 1 needs to be subtracted from prefix_len to account for the null terminator. I'll comment with a fix, and will return in a couple fo days when I can dedicate time to this.

szhorvat · 2026-01-13T10:48:36Z

src/rinterface_extra.c

+ * build procedure, namely on the directory that the compiler is invoked from. */
+static inline const char* simplify_file_path(const char *file) {
+  const char prefix[] = "vendor/cigraph/src/";
+  const size_t prefix_len = sizeof(prefix) / sizeof(prefix[0]);


Suggested change

const size_t prefix_len = sizeof(prefix) / sizeof(prefix[0]);

const size_t prefix_len = sizeof(prefix) / sizeof(prefix[0]) - 1;

maelle · 2026-01-13T10:54:46Z

oops my push hadn't worked... anyhow thanks a ton!!

Run: https://github.com/igraph/rigraph/actions/runs/20954085215

krlmlr

@copilot: Address review. Also create a new test for just the functionality: a function that we can call from R that throws an error with file-line information, and a corresponding test.

krlmlr · 2026-01-22T10:08:14Z

tests/testthat/test-centrality.R

-  expect_snapshot(error = TRUE, {
-    arpack(f, options = list(nev = 2, ncv = 4), sym = TRUE)
-  })
+  expect_snapshot(


Create a new wrapper for expect_snapshot(error = TRUE, transform = ...) . Use where applicable.

Copilot · 2026-01-22T10:10:56Z

@krlmlr I've opened a new pull request, #2522, to work on those changes. Once the pull request is ready, I'll request review from you.

…function (#2522) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com> Co-authored-by: Kirill Müller <krlmlr@users.noreply.github.com>

Run: https://github.com/igraph/rigraph/actions/runs/21313063875

krlmlr

@copilot: Use expect_snapshot_igraph_error() everywhere instead of expect_snapshot(error = TRUE), except in tests/testthat/test-error-formatting.R . Search in tests/testthat for a literal expect_snapshot(error = TRUE first to find many occurrences, then search for error = TRUE and look for a surrounding expect_snapshot() call. Ensure all calls are transformed, except the one where we add a comment to explain why we keep the test. Move Rx_igraph_test_error_with_source() into a separate C file to keep the line number in the error message stable.

Copilot · 2026-01-24T09:48:21Z

@krlmlr I've opened a new pull request, #2528, to work on those changes. Once the pull request is ready, I'll request review from you.

…raph_error()` (#2528) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>

krlmlr · 2026-01-24T10:26:06Z

@copilot: Change expect_snapshot_igraph_error() to also scrub the file name. Update snapshots.

Copilot · 2026-01-24T10:26:13Z

@krlmlr I've opened a new pull request, #2537, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>

krlmlr

@copilot: Let's do one more round.

krlmlr · 2026-01-24T11:03:06Z

tests/testthat/_snaps/games.md

      Error in `sample_degseq()`:
-      ! At vendor/cigraph/src/games/degree_sequence_vl/gengraph_mr-connected.cpp:<linenumber> : Cannot make a connected graph from the given degree sequence. Invalid value
+      ! Cannot make a connected graph from the given degree sequence. Invalid value
+      Source: games/degree_sequence_vl/gengraph_mr-connected.cpp:<linenumber>


Scrub here too.

krlmlr · 2026-01-24T11:03:13Z

tests/testthat/_snaps/aaa-auto.md

      Warning in `connect_neighborhood_impl()`:
-      At vendor/cigraph/src/operators/connect_neighborhood.c:85 : Order smaller than two, graph will be unchanged.
+      Order smaller than two, graph will be unchanged.
+      Source: operators/connect_neighborhood.c:85


krlmlr · 2026-01-24T11:04:19Z

tests/testthat/_snaps/aaa-auto.md

    Condition
      Error in `hrg_sample_impl()`:
-      ! At vendor/cigraph/src/hrg/hrg_types.cc:xx : Assertion failed: n >= 0. This is an unexpected igraph error; please report this as a bug, along with the steps to reproduce it.
+      ! At <file>:<line> : Assertion failed: n >= 0. This is an unexpected igraph error; please report this as a bug, along with the steps to reproduce it.


These and similar errors hint at an error in the wrapping code. Fix.

Copilot · 2026-01-24T11:04:43Z

@krlmlr I've opened a new pull request, #2538, to work on those changes. Once the pull request is ready, I'll request review from you.

Run: https://github.com/igraph/rigraph/actions/runs/21314037440

krlmlr · 2026-01-24T17:22:08Z

Thanks!

feat: improve formatting of error messages from C

2e6b3d0

maelle requested review from schochastics and szhorvat January 6, 2026 11:47

szhorvat reviewed Jan 6, 2026

View reviewed changes

src/rinterface_extra.c Outdated Show resolved Hide resolved

schochastics approved these changes Jan 8, 2026

View reviewed changes

Update src/rinterface_extra.c

9f268d8

Co-authored-by: Szabolcs Horvát <szhorvat@gmail.com>

maelle added 2 commits January 13, 2026 08:56

Merge remote-tracking branch 'origin/main' into better

67565b1

chore: Auto-update from GitHub Actions

63986e8

Run: https://github.com/igraph/rigraph/actions/runs/20950334632

szhorvat reviewed Jan 13, 2026

View reviewed changes

fix @szhorvat

73b4119

chore: Auto-update from GitHub Actions

d273bfe

Run: https://github.com/igraph/rigraph/actions/runs/20954085215

krlmlr reviewed Jan 22, 2026

View reviewed changes

Copilot AI mentioned this pull request Jan 22, 2026

Address review feedback: use helper for error snapshots and add test function #2522

Merged

1 task

Copilot AI mentioned this pull request Jan 22, 2026

docs: Add comprehensive meta-review of open PRs with clickable links #2524

Draft

1 task

Copilot AI and others added 3 commits January 24, 2026 10:37

Merge remote-tracking branch 'origin/main' into better

f41ed10

chore: Auto-update from GitHub Actions

9e4fb9e

Run: https://github.com/igraph/rigraph/actions/runs/21313063875

krlmlr reviewed Jan 24, 2026

View reviewed changes

Copilot AI mentioned this pull request Jan 24, 2026

refactor: standardize error snapshot testing with expect_snapshot_igraph_error() #2528

Merged

1 task

refactor: standardize error snapshot testing with `expect_snapshot_ig…

5a93c98

…raph_error()` (#2528) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>

Copilot AI mentioned this pull request Jan 24, 2026

refactor: scrub file names in expect_snapshot_igraph_error() #2537

Merged

1 task

refactor: scrub file names in expect_snapshot_igraph_error() (#2537)

625e841

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>

krlmlr reviewed Jan 24, 2026

View reviewed changes

Copilot AI mentioned this pull request Jan 24, 2026

fix: scrub file paths in error/warning snapshots and fix spacing #2538

Merged

1 task

krlmlr added 2 commits January 24, 2026 11:06

Merge remote-tracking branch 'origin/main' into better

7773f72

chore: Auto-update from GitHub Actions

a9dfd3a

Run: https://github.com/igraph/rigraph/actions/runs/21314037440

krlmlr merged commit c4a2c33 into main Jan 24, 2026
1 check passed

krlmlr deleted the better branch January 24, 2026 17:22

	const size_t prefix_len = sizeof(prefix) / sizeof(prefix[0]);
	const size_t prefix_len = sizeof(prefix) / sizeof(prefix[0]) - 1;

Uh oh!

Conversation

maelle commented Jan 6, 2026

Uh oh!

szhorvat commented Jan 6, 2026

Uh oh!

Uh oh!

szhorvat commented Jan 6, 2026

Uh oh!

maelle commented Jan 13, 2026

Uh oh!

maelle commented Jan 13, 2026

Uh oh!

szhorvat commented Jan 13, 2026

Uh oh!

szhorvat Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

maelle commented Jan 13, 2026

Uh oh!

krlmlr left a comment

Choose a reason for hiding this comment

Uh oh!

krlmlr Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Jan 22, 2026

Uh oh!

krlmlr left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Jan 24, 2026

Uh oh!

krlmlr commented Jan 24, 2026

Uh oh!

Copilot AI commented Jan 24, 2026

Uh oh!

krlmlr left a comment

Choose a reason for hiding this comment

Uh oh!

krlmlr Jan 24, 2026

Choose a reason for hiding this comment

Uh oh!

krlmlr Jan 24, 2026

Choose a reason for hiding this comment

Uh oh!

krlmlr Jan 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Jan 24, 2026

Uh oh!

Uh oh!

krlmlr commented Jan 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants