test: improve sample_degseq() tests

[maelle]

• ensure we have tests for the examples in docs: fix sample_degseq() example #1297
• put the tests in a test file named like the script.
• remove part of the test that I did not see any need for, see below

  gc <- function(graph) {
    clu <- components(graph)
    induced_subgraph(graph, which(clu$membership == which.max(clu$csize)))
  }

  g <- gc(sample_gnp(1000, 2 / 1000))

  nG <- sample_degseq(degree(g), method = "simple")
  expect_that(degree(nG), equals(degree(g)))

  nG <- sample_degseq(degree(g), method = "vl")
  expect_that(degree(nG), equals(degree(g)))
  expect_true(is_connected(nG))
  expect_true(is_simple(nG))

  #####

  g <- sample_gnp(1000, 1 / 1000)

  nG <- sample_degseq(degree(g), method = "simple")
  expect_that(degree(nG), equals(degree(g)))

  g2 <- sample_gnp(1000, 2 / 1000, directed = TRUE)

  nG2 <- sample_degseq(degree(g, mode = "out"),
    degree(g, mode = "in"),
    method = "simple"
  )
  expect_that(degree(nG, mode = "out"), equals(degree(g, mode = "out")))
  expect_that(degree(nG, mode = "in"), equals(degree(g, mode = "in")))

[aviator-app]

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 PR was merged using Aviator.

---

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.

[maelle]

will rebase once #1297 is merged.

[maelle]

@szhorvat do you feel strongly about any of the test cases I removed? See my first comment.

[szhorvat]

Is the code you quote in the top post what you removed? It's a bit hard to follow what's missing since code was moved between files.

It seems to me that the tests that you quote were all relevant. Why were they removed? I'm a bit lost.

[maelle]

Why were they removed? I'm a bit lost.

Because I couldn't see what they brought based on the tests that are now in there, and they're a bit complicated (for instance, why is there a need to define the function called gc()).

[szhorvat]

These tests verify that the properties that the function guarantees actually hold:

• The degrees of the output graph are the same as the input
• The graph is simple with methods that promise this
• The graph is connected with methods that promise this

I don't mind a refactoring of the test, but all these properties should be tested, ideally for all methods, including both the undirected and directed case.

There is a real risk that a refactoring of this function will accidentally break some of these properties, so it's important to test them.

[szhorvat]

P.S. It'd be good to rename the methods in this function quite soon, see #876. The current names are rather bad and confusing, I can't even remember which properties hold for which method without looking them up ... This is why I didn't list them.

[maelle]

ok, I'll add the tests back. thank you!

[maelle]

@szhorvat in

gc <- function(graph) {
    clu <- components(graph)
    induced_subgraph(graph, which(clu$membership == which.max(clu$csize)))
  }

what could be a more explicit name for gc? it'd help me (and future contributors I guess) understand the test.

[maelle]

this is nearly ready but I'd like to rename the helper function with your feedback. All tests are now in there, and they don't re-use the name "g" and "nG".

[szhorvat]

what could be a more explicit name for gc?

I'm sorry, I'm not paying attention ... "gc" refers to the giant component and actually we have a function for that now! It's called largest_component(). The reason why it is used here is to create a degree sequence that is graphical and has a connected realization. This is done by creating a connected random graph, and taking its degrees.

Let's do g <- largest_component(sample_gnp(1000, 2 / 1000))

[maelle]

mmmh why was this merged before the checks were run?!

[maelle]

@krlmlr did the Aviator settings somehow change?

[krlmlr]

Weird, let's keep an eye on that.