Merge pull request #257 from MaLoefUDS/dev

Fix a bug in the reconstruction of the 'bins' network-attribute from given ranges Reviewed-by: Thomas Bock <[email protected]>
se-sic · Apr 12, 2024 · 6536a4c · 6536a4c
2 parents 1d3d1a3 + db5cdd6
commit 6536a4c
Show file tree

Hide file tree

Showing 4 changed files with 8 additions and 9 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -11,7 +11,7 @@
 - Add new `assert.sparse.matrices.equal` function to compare two sparse matrices for equality for testing purposes (PR #248, 9784cdf12d1497ee122e2ae73b768b8c334210d4, d9f1a8d90e00a634d7caeb5e7f8f262776496838)
 - Add tests for file `util-networks-misc.R` for issue #242 (PR #248, f3202a6f96723d11c170346556d036cf087521c8, 030574b9d0f3435db4032d0e195a3d407fb7244b, 380b02234275127297fcd508772c69db21c216de, 8b803c50d60fc593e4e527a08fd4c2068d801a48, 7335c3dd4d0302b024a66d18701d9800ed3fe806, 6b600df04bec1fe70c272604f274ec5309840e65)
 - Add the possibility to simplify edges of multiple-relation networks into a single edge at all instead of a single edge per relation (PR #250, PR #255, 2105ea89b5227e7c9fa78fea9de1977f2d9e8faa, a34b5bd50351b9ccf3cc45fc323cfa2e84d65ea0, 34516415fed599eba0cc7d3cc4a9acd6b26db252, 78f43514962d7651e6b7a1e80ee22ce012f32535, d310fdc38690f0d701cd32c92112c33f7fdde0ff, 58d77b01ecc6a237104a4e72ee5fb9025efeaaf2)
-- Add tests for network simplification (PR #255, 338b06941eec1c9cfdb121e78ce0d9db6b75da19, 8a6f47bc115c10fbbe4eee21985d97aee5c9dc91, e01908c94eccc4dda5f2b3c0746b0eab0172dc07, 7b6848fb86f69db088ce6ef2bea8315ac94d48f9)
+- Add tests for network simplification (PR #255, 338b06941eec1c9cfdb121e78ce0d9db6b75da19, 8a6f47bc115c10fbbe4eee21985d97aee5c9dc91, e01908c94eccc4dda5f2b3c0746b0eab0172dc07, 7b6848fb86f69db088ce6ef2bea8315ac94d48f9, 666d78444ffcb3bc8b36f2121284e4840176618e)
 - Add `get.bin.dates.from.ranges` function to convert date ranges into bins format (PR #249, a1842e9be46596321ee86860fd87d17a3c88f50f, 858b1812ebfc3194cc6a03c99f3ee7d161d1ca15)
 - Add network simplification to showcase file (PR #255, dc32d44f9db7dfc3cc795ef5d6b86609d6c1936f)
 
@@ -25,7 +25,7 @@
 - Throw an error in `convert.adjacency.matrix.list.to.array` if the function is called with wrong parameters (PR #248, ece2d38b4972745af3a83e06f32317a06465a345, 1a3e510df15f5fa4e920e9fce3e0e162c27cd6d1)
 - Rename `compare.networks` to `assert.networks.equal` to better match the purpose of the function (PR #248, d9f1a8d90e00a634d7caeb5e7f8f262776496838)
 - Explicitly add R version 4.3 to the CI test pipeline (9f346d5bc3cfc553f01e5e80f0bbe51e1dc2b53e)
-- Simplify call chain-, and branching-routes in network-splitting functions and consequently set the `bins` attribute on every output network-split (while minimizing recalculations) (PR #249, a1842e9be46596321ee86860fd87d17a3c88f50f)
+- Simplify call chain-, and branching-routes in network-splitting functions and consequently set the `bins` attribute on every output network-split (while minimizing recalculations) (PR #249, PR #257, a1842e9be46596321ee86860fd87d17a3c88f50f, 8695fbe7f21ccaa3ccd6d1016e754017d387b1fa)
 - Test for the presence and validity of the `bins` attribute on network-, and data-splits (PR #249, c064affcfff2eb170d8bdcb39d837a7ff62b2cbd, 93051ab848ec94de138b0513dac22f6da0d20885)
 
 ### Fixed

diff --git a/tests/test-networks.R b/tests/test-networks.R
@@ -105,7 +105,7 @@ test_that("Simplify basic multi-relational network", {
 
     ## simplify network with simplifying multiple relations into single edges
     network.simplified = simplify.network(network, simplify.multiple.relations = TRUE)
-    expect_identical(igraph::ecount(simplify.network(network.simplified)), 1)
+    expect_identical(igraph::ecount(network.simplified), 1)
     expect_identical(igraph::E(network.simplified)$type[[1]], "Unipartite")
     expect_identical(igraph::E(network.simplified)$relation[[1]], c("cochange", "mail"))
 })
@@ -265,7 +265,7 @@ test_that("Simplify multiple basic multi-relational networks", {
     expect_true(length(networks.simplified) == 2)
     expect_identical(names(networks.simplified), names(networks))
     for (i in 1:2) {
-        expect_identical(igraph::ecount(simplify.network(networks.simplified[[i]])), 1)
+        expect_identical(igraph::ecount(networks.simplified[[i]]), 1)
         expect_identical(igraph::E(networks.simplified[[i]])$type[[1]], "Unipartite")
         expect_identical(igraph::E(networks.simplified[[i]])$relation[[1]], c("cochange", "mail"))
     }

diff --git a/tests/test-split-network-time-based.R b/tests/test-split-network-time-based.R
@@ -206,7 +206,8 @@ patrick::with_parameters_test_that("Split a network time-based (time.period = ..
     expected.bins = get.date.from.string(c("2016-07-12 15:58:59", "2016-07-12 15:59:59",
                                            "2016-07-12 16:00:59", "2016-07-12 16:01:59",
                                            "2016-07-12 16:02:59", "2016-07-12 16:03:59",
-                                           "2016-07-12 16:04:59", "2016-07-12 16:06:33"))
+                                           "2016-07-12 16:04:59", "2016-07-12 16:05:59",
+                                           "2016-07-12 16:06:33"))
     expect_equal(expected.bins, attr(results, "bins"))
 
     ## check networks

diff --git a/util-split.R b/util-split.R
@@ -815,11 +815,9 @@ split.network.time.based.by.ranges = function(network, ranges, remove.isolates =
     )
 
     ## convert ranges to bins
-    bins.starts = sapply(ranges.bounds, function(range) range[1])
-    bins.end = ranges.bounds[[length(ranges.bounds)]][2]
-    bins.date = get.date.from.unix.timestamp(c(bins.starts, bins.end))
+    bins = get.bin.dates.from.ranges(ranges.bounds)
+    attr(nets.split, "bins") = bins
 
-    attr(nets.split, "bins") = bins.date
     return(nets.split)
 }