Merge pull request #105 from bockthom/thomas-updates

Some fixes to README, range construction, vertex attributes, and network-splitting functions

Reviewed-by: Claus Hunsen <[email protected]>

Author: clhunsen

CONTRIBUTING.md

@@ -105,7 +105,8 @@ Install the library using the guide in our [README](README.md) file. Start imple
 In our development process, we pursue the following idea:
 - Each version (i.e., a tag) contains, at least, a major and a minor version in the form `v{major}.{minor}[.{bugfix}]`.
 - The branch `master` should always contain the most recent and complete version.
-- For each version, we maintain a stable branch `v{major}.{minor}-fixes` (e.g., `v3.0-fixes`) containing backported fixes for this particular version. Sometimes, we will annotate such branches with continuous version numbers indicating bugfix releases (e.g., `v3.0.2`).
+- Usually, we release a new version (i.e., a tag) after bugfixes to the most recent version (e.g., when fixing some bugs after release `v3.1`, we introduce a new version `v3.1.1` containing the bugfixes.
+- When fixing some extreme bugs, we maintain a stable branch `v{major}.{minor}-fixes` (e.g., `v2.0-fixes`) for the second last major version containing backported fixes for this particular version (e.g., when we fix a extreme bug in version `v3.1` by introducing version `v3.1.1`, we might backport the fix to major version 2 by maintaining a branch `v2.x-fixes` where `x` denotes the last minor version belonging to major version 2.)
 - The current development will be performed on the branch `dev`, i.e., all incoming pull requests are against this branch.
 
 The current build status is as follows:

NEWS.md

@@ -1,5 +1,20 @@
 # codeface-extraction-r - Changelog
 
+## unversioned
+
+### Added
+
+
+### Changed/Improved
+- Improve performance of several functions used for adding vertex attributes (#102, PR #105)
+- Change default values for aggregation levels for some functions (#102, PR #105)
+- Add missing `remove.isolates` parameter to some more network-splitting functions (011328e881b09bd736dc83475ba7e6cab663bebe)
+- Some minor improvements to the test suite
+
+### Fixed
+- Fix outdated statements in README.md (PR #105)
+- Fix range construction when difference between `start` and `end` is smaller than `time.period` (#103, PR #105, 975ae4d2e1b954d92f945c5853959ff2b3e47083)
+
 
 ## 3.1
 
@@ -33,7 +48,7 @@
 
 ## 3.0.1
 
-## Added
+### Added
 - Add committer data to commit data source (#35, 251cfdbac0ab31584a7fab8bbaf8398a53ae8d11)
 - Add function to delete isolate vertices from a network (5d91ddd89a488212eabf2ce110ec7210fc3c971d)
 

README.md

@@ -26,7 +26,7 @@ It may lead to unpredictable behavior, when you do not do this, as we need to se
 When selecting a version to work with, you should consider the following points:
 - Each version (i.e., a tag) contains, at least, a major and a minor version in the form `v{major}.{minor}[.{bugfix}]`.
 - On the branch `master`, there is always the most recent and complete version.
-- You should always work with the current version on the `master` branch, *unless* there is a branch called `{your_version}-fixes` (e.g., `v3.0-fixes`), then select this one as it contains backported bugfixes for this version. Sometimes, we will annotate such branches with continuous version numbers indicating bugfix releases (e.g., `v3.0.2`).
+- You should always work with the current version on the `master` branch. If you, nentheless, work on a former version, there might be a branch called `{your_version}-fixes` (e.g., `v2.3-fixes`) when we have fixed some extreme bugs in the current version, then select this one as it contains backported bugfixes for the former version. We will backport some very important bugfixes only in special cases and only for the last minor version of the second last major version.
 - If you are confident enough, you can use the `dev` branch.
 
 ### Needed R packages
@@ -261,8 +261,8 @@ Updates to the parameters can be done by calling `NetworkConf$update.variables(.
   * **Note**: This parameter does not affect the original data object, but rather creates a clone.
   * [`TRUE`, *`FALSE`*]
 
-The classes `ProjectData` and `RangeData` hold instances of  the `NetworkConf` class, just pass the object as parameter to the constructor.
-You can also update the object at any time, but as soon as you do so, all cached data of the data object are reset and have to be rebuilt.
+The class `NetworkBuilder` holds an instance of the `NetworkConf` class, just pass the object as parameter to the constructor.
+You can also update the `NetworkConf` object at any time by calling `NetworkBuilder$update.network.conf(...)`, but as soon as you do so, all cached data of the `NetworkBuilder` object are reset and have to be rebuilt.
 
 For more examples, please look in the file `showcase.R`.
 

tests/test-misc.R

@@ -13,10 +13,100 @@
 ##
 ## Copyright 2017 by Felix Prasse <[email protected]>
 ## Copyright 2017-2018 by Claus Hunsen <[email protected]>
-## Copyright 2017 by Thomas Bock <[email protected]>
+## Copyright 2017-2018 by Thomas Bock <[email protected]>
 ## All Rights Reserved.
 
 
+## / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / /
+## Parameter verification --------------------------------------------------
+
+##
+## Match argument or take default.
+##
+
+test_that("Match argument or take default.", {
+
+    ## 1) tests for single choice without default
+    test.function = function(param = c("choice1", "choice2", "choice3")) {
+        param = match.arg.or.default(param)
+        return(param)
+    }
+
+    actual.result = test.function()
+    expected.result = "choice1"
+    expect_equal(actual.result, expected.result, info = "Single choice without default, no choice")
+
+    actual.result = test.function("choice3")
+    expected.result = "choice3"
+    expect_equal(actual.result, expected.result, info = "Single choice without default, one choice")
+
+    expect_error(test.function(c("choice3", "choice1")), info = "Single choice with default, two choices")
+
+    ## 2) tests for single choice with default
+    test.function = function(param = c("choice1", "choice2", "choice3")) {
+        param = match.arg.or.default(param, default = "choice2")
+        return(param)
+    }
+
+    actual.result = test.function()
+    expected.result = "choice2"
+    expect_equal(actual.result, expected.result, info = "Single choice with default, no choice")
+
+    actual.result = test.function("choice3")
+    expected.result = "choice3"
+    expect_equal(actual.result, expected.result, info = "Single choice with default, one choice")
+
+    actual.result = test.function(c("choice3", "choice1"))
+    expected.result = "choice2"
+    expect_equal(actual.result, expected.result, info = "Single choice without default, two choices")
+
+    ## 3) tests for single choice with illegal default
+    test.function = function(param = c("choice1", "choice2", "choice3")) {
+        param = match.arg.or.default(param, default = "c2")
+        return(param)
+    }
+
+    expect_error(test.function(), info = "Single choice with illegal default, no choice")
+    expect_error(test.function(c("choice3", "choice1")), info = "Single choice with illegal default, one choice")
+    expect_error(test.function(c("choice3", "choice1")), info = "Single choice with illegal default, two choices")
+
+    ## 4) tests for multiple choices
+    test.function = function(param = c("choice1", "choice2", "choice3")) {
+        param = match.arg.or.default(param, several.ok = TRUE)
+        return(param)
+    }
+
+    actual.result = test.function()
+    expected.result = c("choice1", "choice2", "choice3")
+    expect_equal(actual.result, expected.result, info = "Multiple choices, no choice")
+
+    actual.result = test.function("choice3")
+    expected.result = "choice3"
+    expect_equal(actual.result, expected.result, info = "Multiple choices, one choice")
+
+    actual.result = test.function(c("choice3", "choice1"))
+    expected.result = c("choice3", "choice1")
+    expect_equal(actual.result, expected.result, info = "Multiple choices, two choices")
+
+    ## 5) tests for multiple choices with ignored default
+    test.function = function(param = c("choice1", "choice2", "choice3")) {
+        param = match.arg.or.default(param, default = "choice2", several.ok = TRUE)
+        return(param)
+    }
+
+    actual.result = test.function()
+    expected.result = c("choice1", "choice2", "choice3")
+    expect_equal(actual.result, expected.result, info = "Multiple choices with ignored default, no choice")
+
+    actual.result = test.function("choice3")
+    expected.result = "choice3"
+    expect_equal(actual.result, expected.result, info = "Multiple choices with ignored default, one choice")
+
+    actual.result = test.function(c("choice3", "choice1"))
+    expected.result = c("choice3", "choice1")
+    expect_equal(actual.result, expected.result, info = "Multiple choices with ignored default, two choices")
+})
+
 ## / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / /
 ## Date handling -----------------------------------------------------------
 
@@ -238,6 +328,53 @@ test_that("Construct cumulative ranges.", {
     ## TODO use expect_identical here? why failing?
 })
 
+##
+## Construct ranges when difference between start and end is smaller than time period.
+##
+
+test_that("Construct ranges when difference between start and end is smaller than time period.", {
+
+    start = ("2015-05-01 01:01:01")
+    start.date = get.date.from.string(start)
+    end = ("2015-05-02 02:02:02")
+    end.date = get.date.from.string(end)
+    end.including = end.date + 1
+
+    ## expected results (equal for consecutive, cumulative, and overlapping range construction)
+    expected.formatted = c("2015-05-01 01:01:01-2015-05-02 02:02:03")
+    expected.raw = lapply(expected.formatted, get.range.bounds)
+    names(expected.raw) = expected.formatted
+
+    ## consecutive
+    ## 1) formatted
+    result.formatted = construct.consecutive.ranges(start, end, time.period = "1 week", raw = FALSE)
+    expect_identical(result.formatted, expected.formatted, info = "Consecutive range (formatted).")
+    ## 2) raw
+    result.raw = construct.consecutive.ranges(start, end, time.period = "1 week", raw = TRUE)
+    expect_equal(result.raw, expected.raw, info = "Consecutive range (raw).")
+    ## TODO use expect_identical here? why failing?
+
+    ## cumulative
+    ## 1) formatted
+    result.formatted = construct.cumulative.ranges(start, end, time.period = "1 week", raw = FALSE)
+    expect_identical(result.formatted, expected.formatted, info = "Cumulative range (formatted).")
+    ## 2) raw
+    result.raw = construct.cumulative.ranges(start, end, time.period = "1 week", raw = TRUE)
+    expect_equal(result.raw, expected.raw, info = "Cumulative range (raw).")
+    ## TODO use expect_identical here? why failing?
+
+    ## overlapping
+    ## 1) formatted
+    result.formatted = construct.overlapping.ranges(start, end, time.period = "1 week",
+                                                    overlap = "1 day", raw = FALSE)
+    expect_identical(result.formatted, expected.formatted, info = "Overlapping range (formatted).")
+    ## 2) raw
+    result.raw = construct.overlapping.ranges(start, end, time.period = "1 week",
+                                              overlap = "5 days", raw = TRUE)
+    expect_equal(result.raw, expected.raw, info = "Overlapping range (raw).")
+    ## TODO use expect_identical here? why failing?
+})
+
 ##
 ## Aggregate ranges.
 ##

tests/test-network-covariates.R

@@ -491,16 +491,33 @@ test_that("Test add.vertex.attribute.artifact.editor.count", {
 
     networks.and.data = get.network.covariates.test.networks("artifact")
 
-    expected.attributes = network.covariates.test.build.expected(list(1L), list(1L), list(3L, 1L))
+    expected.attributes = list(
+        range = network.covariates.test.build.expected(
+            list(1L), list(1L), list(3L, 1L)),
+        cumulative = network.covariates.test.build.expected(
+            list(1L), list(2L), list(3L, 1L)),
+        all.ranges = network.covariates.test.build.expected(
+            list(2L), list(2L), list(3L, 1L)),
+        project.cumulative = network.covariates.test.build.expected(
+            list(1L), list(2L), list(3L, 1L)),
+        project.all.ranges = network.covariates.test.build.expected(
+            list(2L), list(2L), list(3L, 1L)),
+        complete = network.covariates.test.build.expected(
+            list(2L), list(2L), list(3L, 1L))
+    )
 
     ## Test
 
-    networks.with.attr = add.vertex.attribute.artifact.editor.count(networks.and.data[["networks"]],
-                                                                    networks.and.data[["project.data"]])
+    lapply(AGGREGATION.LEVELS, function(level) {
+        networks.with.attr = add.vertex.attribute.artifact.editor.count(
+            networks.and.data[["networks"]], networks.and.data[["project.data"]],
+            aggregation.level = level
+        )
 
-    actual.attributes = lapply(networks.with.attr, igraph::get.vertex.attribute, name = "editor.count")
+        actual.attributes = lapply(networks.with.attr, igraph::get.vertex.attribute, name = "editor.count")
 
-    expect_identical(expected.attributes, actual.attributes)
+        expect_equal(expected.attributes[[level]], actual.attributes)
+    })
 })
 
 #' Test the add.vertex.attribute.artifact.first.occurrence method
@@ -510,20 +527,45 @@ test_that("Test add.vertex.attribute.artifact.first.occurrence", {
 
     networks.and.data = get.network.covariates.test.networks("artifact")
 
-    expected.attributes = network.covariates.test.build.expected(
-        dateList("2016-07-12 15:58:59 UTC"), dateList("2016-07-12 16:00:45 UTC"),
-        dateList("2016-07-12 16:05:41 UTC", "2016-07-12 16:06:32 UTC")
+    expected.attributes = list(
+        range = network.covariates.test.build.expected(
+            dateList("2016-07-12 15:58:59 UTC"), dateList("2016-07-12 16:00:45 UTC"),
+            dateList("2016-07-12 16:05:41 UTC", "2016-07-12 16:06:32 UTC")
+        ),
+        cumulative = network.covariates.test.build.expected(
+            dateList("2016-07-12 15:58:59 UTC"), dateList("2016-07-12 15:58:59 UTC"),
+            dateList("2016-07-12 16:05:41 UTC", "2016-07-12 16:06:32 UTC")
+        ),
+        all.ranges = network.covariates.test.build.expected(
+            dateList("2016-07-12 15:58:59 UTC"), dateList("2016-07-12 15:58:59 UTC"),
+            dateList("2016-07-12 16:05:41 UTC", "2016-07-12 16:06:32 UTC")
+        ),
+        project.cumulative = network.covariates.test.build.expected(
+            dateList("2016-07-12 15:58:59 UTC"), dateList("2016-07-12 15:58:59 UTC"),
+            dateList("2016-07-12 16:05:41 UTC", "2016-07-12 16:06:32 UTC")
+        ),
+        project.all.ranges = network.covariates.test.build.expected(
+            dateList("2016-07-12 15:58:59 UTC"), dateList("2016-07-12 15:58:59 UTC"),
+            dateList("2016-07-12 16:05:41 UTC", "2016-07-12 16:06:32 UTC")
+        ),
+        complete = network.covariates.test.build.expected(
+            dateList("2016-07-12 15:58:59 UTC"), dateList("2016-07-12 15:58:59 UTC"),
+            dateList("2016-07-12 16:05:41 UTC", "2016-07-12 16:06:32 UTC")
+        )
     )
 
     ## Test
 
-    networks.with.attr = add.vertex.attribute.artifact.first.occurrence(
-        networks.and.data[["networks"]], networks.and.data[["project.data"]]
-    )
+    lapply(AGGREGATION.LEVELS, function(level) {
+        networks.with.attr = add.vertex.attribute.artifact.first.occurrence(
+            networks.and.data[["networks"]], networks.and.data[["project.data"]],
+            aggregation.level = level
+        )
 
-    actual.attributes = lapply(networks.with.attr, igraph::get.vertex.attribute, name = "first.occurrence")
+        actual.attributes = lapply(networks.with.attr, igraph::get.vertex.attribute, name = "first.occurrence")
 
-    expect_equal(expected.attributes, actual.attributes)
+        expect_equal(expected.attributes[[level]], actual.attributes)
+    })
 })
 
 #' Test the add.vertex.attribute.artifact.change.count method
@@ -533,15 +575,31 @@ test_that("Test add.vertex.attribute.artifact.change.count", {
 
     networks.and.data = get.network.covariates.test.networks("artifact")
 
-    expected.attributes = network.covariates.test.build.expected(list(1L), list(1L), list(3L, 1L))
+    expected.attributes = list(
+        range = network.covariates.test.build.expected(
+            list(1L), list(1L), list(3L, 1L)),
+        cumulative = network.covariates.test.build.expected(
+            list(1L), list(2L), list(3L, 1L)),
+        all.ranges = network.covariates.test.build.expected(
+            list(2L), list(2L), list(3L, 1L)),
+        project.cumulative = network.covariates.test.build.expected(
+            list(1L), list(2L), list(3L, 1L)),
+        project.all.ranges = network.covariates.test.build.expected(
+            list(2L), list(2L), list(3L, 1L)),
+        complete = network.covariates.test.build.expected(
+            list(2L), list(2L), list(3L, 1L))
+    )
 
     ## Test
 
-    networks.with.attr = add.vertex.attribute.artifact.change.count(
-        networks.and.data[["networks"]], networks.and.data[["project.data"]]
-    )
+    lapply(AGGREGATION.LEVELS, function(level) {
+        networks.with.attr = add.vertex.attribute.artifact.change.count(
+            networks.and.data[["networks"]], networks.and.data[["project.data"]],
+            aggregation.level = level
+        )
 
-    actual.attributes = lapply(networks.with.attr, igraph::get.vertex.attribute, name = "change.count")
+        actual.attributes = lapply(networks.with.attr, igraph::get.vertex.attribute, name = "change.count")
 
-    expect_identical(expected.attributes, actual.attributes)
+        expect_equal(expected.attributes[[level]], actual.attributes)
+    })
 })