From 5868529aab815223d906083c8cdd35f126cb1507 Mon Sep 17 00:00:00 2001 From: Vince Reuter Date: Mon, 2 Dec 2024 16:00:02 +0100 Subject: [PATCH] require groups of regional timepoints (for trace merger) to be named; close #383 --- build.sbt | 1 + project/Dependencies.scala | 8 +- .../scala/ImagingRoundsConfiguration.scala | 10 +- ...e__blank_time_in_tracing_merge_groups.json | 2 +- ...e__locus_time_in_tracing_merge_groups.json | 2 +- ...imes_for_regional_times_to_merge__384.json | 2 +- ...mple__legitimate_tracing_merge_groups.json | 2 +- ..._for_trace_merge_is_only_within_group.json | 2 +- ...undsConfigurationExamplesParsability.scala | 21 ++- ...RoiMergeSectionOfImagingRoundsConfig.scala | 156 ++++++++++++------ 10 files changed, 134 insertions(+), 72 deletions(-) diff --git a/build.sbt b/build.sbt index a2fd826b..c13ab6c9 100644 --- a/build.sbt +++ b/build.sbt @@ -77,6 +77,7 @@ lazy val root = (project in file(".")) gerlibs ++ logging ++ Seq( // only for tests + catsLaws, gerlibTesting, ironScalacheck, scalaCsv, diff --git a/project/Dependencies.scala b/project/Dependencies.scala index 9fe7cdca..97a1e614 100644 --- a/project/Dependencies.scala +++ b/project/Dependencies.scala @@ -2,6 +2,11 @@ import sbt._ /** Dependencies for the project */ object Dependencies { + /** Get a typelevel cats dependency specification. */ + object Cats { + def getModuleId(name: String) = "org.typelevel" %% s"cats-$name" % "2.12.0" + } + /** gerlichlab dependency declaration helper */ object Gerlib { def getModuleId(name: String) = groupId %% artifact(name) % version @@ -37,7 +42,7 @@ object Dependencies { lazy val scalatestVersion = "3.2.18" /* core libraries */ - lazy val catsCore = "org.typelevel" %% "cats-core" % "2.12.0" + lazy val catsCore = Cats.getModuleId("core") lazy val gerlibs = Seq( "graph", "io", @@ -69,6 +74,7 @@ object Dependencies { lazy val uPickle = HaoyiJson.getModuleId("upickle") /* testing-related dependencies */ + lazy val catsLaws = Cats.getModuleId("laws") lazy val gerlibTesting = Gerlib.getModuleId("testing") lazy val scalacheck = "org.scalacheck" %% "scalacheck" % "1.18.0" lazy val scalactic = "org.scalactic" %% "scalactic" % scalatestVersion diff --git a/src/main/scala/ImagingRoundsConfiguration.scala b/src/main/scala/ImagingRoundsConfiguration.scala index 3b73b30d..dc8f38b9 100644 --- a/src/main/scala/ImagingRoundsConfiguration.scala +++ b/src/main/scala/ImagingRoundsConfiguration.scala @@ -520,6 +520,7 @@ object ImagingRoundsConfiguration extends LazyLogging: /** How to redefine trace IDs and filter ROIs on the basis of proximity to one another */ final case class TraceIdDefinitionRule( + name: String, mergeGroup: ProximityGroup[EuclideanDistance.Threshold, ImagingTimepoint], requirement: RoiPartnersRequirementType, ) @@ -567,16 +568,17 @@ object ImagingRoundsConfiguration extends LazyLogging: .flatMap{ (strictness, maybeThreshold, maybeRequirementType) => kvPairs.get(groupsKey) .toRight(s"Missing key for groups: $groupsKey") - .flatMap(_.arrOpt.toRight(s"Can't parse groups (from '$groupsKey') as array-like")) - .flatMap(_.toList.toNel.toRight(s"Groups (from '$groupsKey') is empty")) + .flatMap(_.objOpt.toRight(s"Can't parse groups (from '$groupsKey') as map-like")) + .flatMap(_.toList.toNel.toRight(s"Data for groups (from '$groupsKey') is empty")) .leftMap(NonEmptyList.one) - .flatMap(_.nonEmptyTraverse(v => parseGroupMembersSimple(v).leftMap(NonEmptyList.one))) + .flatMap(_.nonEmptyTraverse{ (k, v) => parseGroupMembersSimple(v).bimap(NonEmptyList.one, k -> _) }) .flatMap(groups => (maybeThreshold, maybeRequirementType) match { case (None, None) => NonEmptyList.one("Missing threshold and requirement type for ROI merge").asLeft case (Some(threshold), None) => NonEmptyList.one("Missing requirement type for ROI merge").asLeft case (None, Some(reqType)) => NonEmptyList.one("Missing threshold for ROI merge").asLeft case (Some(threshold), Some(reqType)) => - groups.map{ g => TraceIdDefinitionRule( + groups.map{ (k, g) => TraceIdDefinitionRule( + k, ProximityGroup(threshold, g), reqType, ) diff --git a/src/test/resources/TestImagingRoundsConfiguration/DifferentTimepointRegionalMergeForTracing/bad_example__blank_time_in_tracing_merge_groups.json b/src/test/resources/TestImagingRoundsConfiguration/DifferentTimepointRegionalMergeForTracing/bad_example__blank_time_in_tracing_merge_groups.json index 5efc162a..d9f17d0b 100644 --- a/src/test/resources/TestImagingRoundsConfiguration/DifferentTimepointRegionalMergeForTracing/bad_example__blank_time_in_tracing_merge_groups.json +++ b/src/test/resources/TestImagingRoundsConfiguration/DifferentTimepointRegionalMergeForTracing/bad_example__blank_time_in_tracing_merge_groups.json @@ -6,7 +6,7 @@ "discardRoisNotInGroupsOfInterest": true, "distanceThreshold": 2000, "requirementType": "Conjunctive", - "groups": [[0, 7], [8, 9]] + "groups": {"A": [0, 7], "B": [8, 9]} }, "imagingRounds": [ {"time": 0, "name": "pre_image", "isBlank": true}, diff --git a/src/test/resources/TestImagingRoundsConfiguration/DifferentTimepointRegionalMergeForTracing/bad_example__locus_time_in_tracing_merge_groups.json b/src/test/resources/TestImagingRoundsConfiguration/DifferentTimepointRegionalMergeForTracing/bad_example__locus_time_in_tracing_merge_groups.json index 009025bc..f1c6e341 100644 --- a/src/test/resources/TestImagingRoundsConfiguration/DifferentTimepointRegionalMergeForTracing/bad_example__locus_time_in_tracing_merge_groups.json +++ b/src/test/resources/TestImagingRoundsConfiguration/DifferentTimepointRegionalMergeForTracing/bad_example__locus_time_in_tracing_merge_groups.json @@ -6,7 +6,7 @@ "discardRoisNotInGroupsOfInterest": true, "distanceThreshold": 2000, "requirementType": "Conjunctive", - "groups": [[5, 7], [8, 9]] + "groups": {"A": [5, 7], "B": [8, 9]} }, "imagingRounds": [ {"time": 0, "name": "pre_image", "isBlank": true}, diff --git a/src/test/resources/TestImagingRoundsConfiguration/DifferentTimepointRegionalMergeForTracing/bad_example__overlap_locus_times_for_regional_times_to_merge__384.json b/src/test/resources/TestImagingRoundsConfiguration/DifferentTimepointRegionalMergeForTracing/bad_example__overlap_locus_times_for_regional_times_to_merge__384.json index 58a3b24d..35e18702 100644 --- a/src/test/resources/TestImagingRoundsConfiguration/DifferentTimepointRegionalMergeForTracing/bad_example__overlap_locus_times_for_regional_times_to_merge__384.json +++ b/src/test/resources/TestImagingRoundsConfiguration/DifferentTimepointRegionalMergeForTracing/bad_example__overlap_locus_times_for_regional_times_to_merge__384.json @@ -6,7 +6,7 @@ "discardRoisNotInGroupsOfInterest": true, "distanceThreshold": 2000, "requirementType": "Conjunctive", - "groups": [[6, 7], [8, 9]] + "groups": {"A": [6, 7], "B": [8, 9]} }, "imagingRounds": [ {"time": 0, "name": "pre_image", "isBlank": true}, diff --git a/src/test/resources/TestImagingRoundsConfiguration/DifferentTimepointRegionalMergeForTracing/good_example__legitimate_tracing_merge_groups.json b/src/test/resources/TestImagingRoundsConfiguration/DifferentTimepointRegionalMergeForTracing/good_example__legitimate_tracing_merge_groups.json index 5feefe8c..f85578ee 100644 --- a/src/test/resources/TestImagingRoundsConfiguration/DifferentTimepointRegionalMergeForTracing/good_example__legitimate_tracing_merge_groups.json +++ b/src/test/resources/TestImagingRoundsConfiguration/DifferentTimepointRegionalMergeForTracing/good_example__legitimate_tracing_merge_groups.json @@ -6,7 +6,7 @@ "discardRoisNotInGroupsOfInterest": true, "distanceThreshold": 2000, "requirementType": "Conjunctive", - "groups": [[6, 7], [8, 9]] + "groups": {"A": [6, 7], "B": [8, 9]} }, "imagingRounds": [ {"time": 0, "name": "pre_image", "isBlank": true}, diff --git a/src/test/resources/TestImagingRoundsConfiguration/DifferentTimepointRegionalMergeForTracing/good_example__locus_time_overlap_prohibition_for_trace_merge_is_only_within_group.json b/src/test/resources/TestImagingRoundsConfiguration/DifferentTimepointRegionalMergeForTracing/good_example__locus_time_overlap_prohibition_for_trace_merge_is_only_within_group.json index aaa0d4f7..8cec4e4b 100644 --- a/src/test/resources/TestImagingRoundsConfiguration/DifferentTimepointRegionalMergeForTracing/good_example__locus_time_overlap_prohibition_for_trace_merge_is_only_within_group.json +++ b/src/test/resources/TestImagingRoundsConfiguration/DifferentTimepointRegionalMergeForTracing/good_example__locus_time_overlap_prohibition_for_trace_merge_is_only_within_group.json @@ -6,7 +6,7 @@ "discardRoisNotInGroupsOfInterest": true, "distanceThreshold": 2000, "requirementType": "Conjunctive", - "groups": [[6, 7], [8, 9]] + "groups": {"A": [6, 7], "B": [8, 9]} }, "imagingRounds": [ {"time": 0, "name": "pre_image", "isBlank": true}, diff --git a/src/test/scala/TestImagingRoundsConfigurationExamplesParsability.scala b/src/test/scala/TestImagingRoundsConfigurationExamplesParsability.scala index 93027190..65c4e5c0 100644 --- a/src/test/scala/TestImagingRoundsConfigurationExamplesParsability.scala +++ b/src/test/scala/TestImagingRoundsConfigurationExamplesParsability.scala @@ -215,14 +215,16 @@ class TestImagingRoundsConfigurationExamplesParsability extends AnyFunSuite with val expDistance = import io.github.iltotore.iron.autoRefine EuclideanDistance.Threshold(NonnegativeReal(2000)) - NonEmptyList.of(Set(6, 7), Set(8, 9)) - .map(_.map(ImagingTimepoint.unsafe).pipe(AtLeast2.unsafe)) - .map{ g => - TraceIdDefinitionRule( - ProximityGroup(expDistance, g), - RoiPartnersRequirementType.Conjunctive, - ) - } + NonEmptyList.of( + "A" -> MergeBag.unsafe(Set(6, 7)), + "B" -> MergeBag.unsafe(Set(8, 9)), + ).map{ (k, g) => + TraceIdDefinitionRule( + k, + ProximityGroup(expDistance, g), + RoiPartnersRequirementType.Conjunctive, + ) + } Table( ("configFileName", "expectedMergeRules"), ("good_example__legitimate_tracing_merge_groups.json", expRules), @@ -287,6 +289,9 @@ class TestImagingRoundsConfigurationExamplesParsability extends AnyFunSuite with } } + object MergeBag: + def unsafe = (_: Set[Int]).map(ImagingTimepoint.unsafe).pipe(AtLeast2.unsafe) + private def checkParseFailure(configFile: os.Path, expectedMessage: String, check: (String, String) => Boolean = cats.Eq[String].eqv) = ImagingRoundsConfiguration.fromJsonFile(configFile) match { case Left(messages) => diff --git a/src/test/scala/TestParseDifferentTimepointsRoiMergeSectionOfImagingRoundsConfig.scala b/src/test/scala/TestParseDifferentTimepointsRoiMergeSectionOfImagingRoundsConfig.scala index aff51898..e007206e 100644 --- a/src/test/scala/TestParseDifferentTimepointsRoiMergeSectionOfImagingRoundsConfig.scala +++ b/src/test/scala/TestParseDifferentTimepointsRoiMergeSectionOfImagingRoundsConfig.scala @@ -1,6 +1,7 @@ package at.ac.oeaw.imba.gerlich.looptrace import cats.data.* +import cats.laws.discipline.arbitrary.given import cats.syntax.all.* import org.scalacheck.* import org.scalatest.funsuite.AnyFunSuite @@ -11,6 +12,10 @@ import at.ac.oeaw.imba.gerlich.gerlib.collections.AtLeast2 import at.ac.oeaw.imba.gerlich.gerlib.collections.AtLeast2.given import at.ac.oeaw.imba.gerlich.gerlib.collections.AtLeast2.syntax.* import at.ac.oeaw.imba.gerlich.gerlib.geometry.EuclideanDistance +import at.ac.oeaw.imba.gerlich.gerlib.imaging.ImagingTimepoint +import at.ac.oeaw.imba.gerlich.gerlib.imaging.instances.all.given +import at.ac.oeaw.imba.gerlich.gerlib.json.JsonValueWriter +import at.ac.oeaw.imba.gerlich.gerlib.numeric.{ NonnegativeReal, PositiveInt } import at.ac.oeaw.imba.gerlich.gerlib.numeric.instances.all.given import at.ac.oeaw.imba.gerlich.gerlib.syntax.all.* import at.ac.oeaw.imba.gerlich.gerlib.testing.instances.all.given @@ -20,8 +25,7 @@ import at.ac.oeaw.imba.gerlich.looptrace.ImagingRoundsConfiguration.{ TraceIdDefinitionRulesSet, } import at.ac.oeaw.imba.gerlich.looptrace.ImagingRoundsConfiguration.TraceIdDefinitionRule -import at.ac.oeaw.imba.gerlich.gerlib.numeric.NonnegativeReal -import at.ac.oeaw.imba.gerlich.gerlib.imaging.ImagingTimepoint +import at.ac.oeaw.imba.gerlich.looptrace.instances.regionId.given /** Tests for the parsing of the section about merging ROIs from different timepoints */ class TestParseDifferentTimepointsRoiMergeSectionOfImagingRoundsConfig extends @@ -31,59 +35,34 @@ class TestParseDifferentTimepointsRoiMergeSectionOfImagingRoundsConfig extends ImagingRoundsConfigurationSuite, should.Matchers: test("Parse succeeds when all parts are present and correctly structured"): - import at.ac.oeaw.imba.gerlich.gerlib.imaging.instances.all.given - import at.ac.oeaw.imba.gerlich.looptrace.instances.regionId.given - given noShrink[A]: Shrink[A] = Shrink.shrinkAny[A] - - def genGroup(using arbTime: Arbitrary[ImagingTimepoint]): Gen[AtLeast2[Set, ImagingTimepoint]] = - Gen.choose(2, 10) - .flatMap(n => Gen.containerOfN[Set, ImagingTimepoint](n, arbTime.arbitrary)) - .suchThat(_.size >= 2) - .map{ g => - AtLeast2.either(g).fold( - msg => throw new Exception(s"Error building group for test: $msg"), - identity - ) - } - - def genGroupingOfOne(using Arbitrary[ImagingTimepoint]): Gen[NonEmptyList[AtLeast2[Set, ImagingTimepoint]]] = - genGroup.map(NonEmptyList.one) - - def genGroupingOfTwo: Gen[NonEmptyList[AtLeast2[Set, ImagingTimepoint]]] = - val indices = (0 to 10).map(ImagingTimepoint.unsafeLift) - for { - ts1 <- Gen.choose(2, 5) - .flatMap(n => Gen.pick(n, indices)) - .map(g => AtLeast2.unsafe(g.toSet)) - ts2 <- Gen.choose(2, indices.size - ts1.size) - .flatMap(n => Gen.pick(n, indices.toSet -- ts1.toSet)) - .map(g => AtLeast2.unsafe(g.toSet)) - } yield NonEmptyList.of(ts1, ts2) - - given arbGrouping(using Arbitrary[ImagingTimepoint]): Arbitrary[NonEmptyList[AtLeast2[Set, ImagingTimepoint]]] = - Arbitrary{ Gen.oneOf(genGroupingOfOne, genGroupingOfTwo) } - forAll: (threshold: EuclideanDistance.Threshold, requirement: RoiPartnersRequirementType, groups: NonEmptyList[AtLeast2[Set, ImagingTimepoint]], strict: Boolean) => - val groupsJson = groups.map(ids => "[" ++ ids.toList.map(_.show_).mkString(", ") ++ "]").mkString_(", ") - val json = s"""{"discardRoisNotInGroupsOfInterest": $strict, "distanceThreshold": ${threshold.get.show_}, "requirementType": "${requirement}", "groups": [${groupsJson}]}""" + forAll (minSuccessful(1000)): ( + threshold: EuclideanDistance.Threshold, + requirement: RoiPartnersRequirementType, + groups: NonEmptyMap[String, AtLeast2[Set, ImagingTimepoint]], + strict: Boolean, + ) => + val groupsJson = makeGroupsJson(groups) + val json = s"""{"discardRoisNotInGroupsOfInterest": $strict, "distanceThreshold": ${threshold.get.show_}, "requirementType": "${requirement}", "groups": {$groupsJson}}""" TraceIdDefinitionRulesSet.fromJson(json) match { case Left(messages) => fail(s"${messages.length} error(s) decoding JSON: ${messages.mkString_("; ")}" ++ "\n" ++ json) case Right(parsedResult) => - val expGroups = groups.map{ g => TraceIdDefinitionRule(ProximityGroup(threshold, g), requirement) } + val expGroups = groups.toNel.map{ (k, g) => TraceIdDefinitionRule(k, ProximityGroup(threshold, g), requirement) } parsedResult shouldEqual (expGroups, strict) } - + test("Simple single-group example parses as expected"): val strict = true - val json = s"""{"discardRoisNotInGroupsOfInterest": $strict, "distanceThreshold": 5, "requirementType": "Conjunctive", "groups": [[9, 10]]}""" + val json = s"""{"discardRoisNotInGroupsOfInterest": $strict, "distanceThreshold": 5, "requirementType": "Conjunctive", "groups": {"A": [9, 10]}}""" TraceIdDefinitionRulesSet.fromJson(json) match { case Left(messages) => fail(s"${messages.length} error(s) decoding JSON: ${messages.mkString_("; ")}" ++ "\n" ++ json) case Right(parsedResult) => import io.github.iltotore.iron.autoRefine val expectation = TraceIdDefinitionRule( + "A", ProximityGroup( EuclideanDistance.Threshold(5: NonnegativeReal), AtLeast2.unsafe(Set(9, 10).map(ImagingTimepoint.unsafeLift)) @@ -98,25 +77,41 @@ class TestParseDifferentTimepointsRoiMergeSectionOfImagingRoundsConfig extends import io.github.iltotore.iron.autoRefine EuclideanDistance.Threshold(5: NonnegativeReal) val strict = true - val json = s"""{"discardRoisNotInGroupsOfInterest": $strict, "distanceThreshold": ${threshold.get.show_}, "requirementType": "Conjunctive", "groups": [[1, 0], [9, 10]]}""" + val json = s"""{"discardRoisNotInGroupsOfInterest": $strict, "distanceThreshold": ${threshold.get.show_}, "requirementType": "Conjunctive", "groups": {"A": [1, 0], "B": [9, 10]}}""" TraceIdDefinitionRulesSet.fromJson(json) match { case Left(messages) => fail(s"${messages.length} error(s) decoding JSON: ${messages.mkString_("; ")}" ++ "\n" ++ json) case Right(parsedResult) => val exp1 = TraceIdDefinitionRule( + "A", ProximityGroup(threshold, AtLeast2.unsafe(Set(1, 0).map(ImagingTimepoint.unsafeLift))), RoiPartnersRequirementType.Conjunctive, ) val exp2 = TraceIdDefinitionRule( + "B", ProximityGroup(threshold, AtLeast2.unsafe(Set(9, 10).map(ImagingTimepoint.unsafeLift))), RoiPartnersRequirementType.Conjunctive, ) parsedResult shouldEqual (NonEmptyList.of(exp1, exp2), strict) } + test("Each group must be named (#383)"): + forAll: (strict: Boolean, groups: NonEmptyMap[String, AtLeast2[Set, ImagingTimepoint]], rawThreshold: PositiveInt, reqType: RoiPartnersRequirementType) => + val fullJson = + val unnamedGroupsJson = groups.map{ g => s"[${g.toList.map(_.show_).mkString(", ")}]" }.mkString_(", ") + s"""{"discardRoisNotInGroupsOfInterest": $strict, "distanceThreshold": ${rawThreshold.show_}, "requirementType": \"$reqType\", "groups": [$unnamedGroupsJson]}""" + TraceIdDefinitionRulesSet.fromJson(fullJson) match { + case Left(messages) => messages.toList match { + case msg :: Nil => + msg shouldEqual "Can't parse groups (from 'groups') as map-like" + case _ => fail(s"Expected exactly 1 error message but got ${messages.length}: ${messages.mkString_("; ")}") + } + case Right(_) => fail("Expected parse failure but got success") + } + test("Without requirement type, no parse"): TraceIdDefinitionRulesSet.fromJson( - s"""{"discardRoisNotInGroupsOfInterest": true, "distanceThreshold": 5, "groups": [[9, 10]]}""" + s"""{"discardRoisNotInGroupsOfInterest": true, "distanceThreshold": 5, "groups": {"A": [9, 10]}}""" ) match { case Left(messages) => messages.count(_.contains("Missing requirement type for ROI merge")) shouldEqual 1 @@ -126,7 +121,7 @@ class TestParseDifferentTimepointsRoiMergeSectionOfImagingRoundsConfig extends test("Without threshold, no parse"): TraceIdDefinitionRulesSet.fromJson( - s"""{"discardRoisNotInGroupsOfInterest": true, "requirementType": "Conjunctive", "groups": [[9, 10]]}""" + s"""{"discardRoisNotInGroupsOfInterest": true, "requirementType": "Conjunctive", "groups": {"A": [9, 10]}}""" ) match { case Left(messages) => messages.count(_.contains("Missing threshold for ROI merge")) shouldEqual 1 @@ -136,7 +131,7 @@ class TestParseDifferentTimepointsRoiMergeSectionOfImagingRoundsConfig extends test("Threshold must be nonnegative"): TraceIdDefinitionRulesSet.fromJson( - s"""{"discardRoisNotInGroupsOfInterest": true, "distanceThreshold": -1, "requirementType": "Conjunctive", "groups": [[9, 10]]}""" + s"""{"discardRoisNotInGroupsOfInterest": true, "distanceThreshold": -1, "requirementType": "Conjunctive", "groups": {"A": [9, 10]}}""" ) match { case Left(messages) => messages.count(_.contains("!(Should be strictly negative)")) shouldEqual 1 @@ -146,7 +141,7 @@ class TestParseDifferentTimepointsRoiMergeSectionOfImagingRoundsConfig extends test("Each grouping member must have at least two members"): TraceIdDefinitionRulesSet.fromJson( - s"""{"discardRoisNotInGroupsOfInterest": true, "distanceThreshold": 5, "requirementType": "Conjunctive", "groups": [[9, 10], [0]]}""" + s"""{"discardRoisNotInGroupsOfInterest": true, "distanceThreshold": 5, "requirementType": "Conjunctive", "groups": {"A": [9, 10], "B": [0]}}""" ) match { case Left(messages) => messages.count(_.contains("Should have a minimum length of 2")) shouldEqual 1 @@ -156,7 +151,7 @@ class TestParseDifferentTimepointsRoiMergeSectionOfImagingRoundsConfig extends test("No member may be repeated within a group"): TraceIdDefinitionRulesSet.fromJson( - s"""{"discardRoisNotInGroupsOfInterest": true, "distanceThreshold": 5, "requirementType": "Conjunctive", "groups": [[9, 10, 9], [0, 1]]}""" + s"""{"discardRoisNotInGroupsOfInterest": true, "distanceThreshold": 5, "requirementType": "Conjunctive", "groups": {"A": [9, 10, 9], "B": [0, 1]}}""" ) match { case Left(messages) => messages.count(_.contains("2 unique value(s), but 3 total")) shouldEqual 1 @@ -166,7 +161,7 @@ class TestParseDifferentTimepointsRoiMergeSectionOfImagingRoundsConfig extends test("No member may be repeated between groups"): TraceIdDefinitionRulesSet.fromJson( - s"""{"discardRoisNotInGroupsOfInterest": true, "distanceThreshold": 5, "requirementType": "Conjunctive", "groups": [[9, 10], [0, 9, 1]]}""" + s"""{"discardRoisNotInGroupsOfInterest": true, "distanceThreshold": 5, "requirementType": "Conjunctive", "groups": {"A": [9, 10], "B": [0, 9, 1]}}""" ) match { case Left(messages) => messages.count(_.contains("repeated item(s) in merge rules")) shouldEqual 1 @@ -176,7 +171,7 @@ class TestParseDifferentTimepointsRoiMergeSectionOfImagingRoundsConfig extends test("Requirement type must be one of the fixed values"): TraceIdDefinitionRulesSet.fromJson( - s"""{"discardRoisNotInGroupsOfInterest": true, "distanceThreshold": 5, "requirementType": "NotARealValue", "groups": [[9, 10]]}""" + s"""{"discardRoisNotInGroupsOfInterest": true, "distanceThreshold": 5, "requirementType": "NotARealValue", "groups": {"A": [9, 10]}}""" ) match { case Left(messages) => messages.count(_.contains("Can't parse value for 'requirementType': NotARealValue")) shouldEqual 1 @@ -186,7 +181,7 @@ class TestParseDifferentTimepointsRoiMergeSectionOfImagingRoundsConfig extends test("With neither threshold nor requirement type, no parse"): TraceIdDefinitionRulesSet.fromJson( - s"""{"discardRoisNotInGroupsOfInterest": true, "groups": [[9, 10]]}""" + s"""{"discardRoisNotInGroupsOfInterest": true, "groups": {"A": [9, 10]}}""" ) match { case Left(messages) => messages.count(_.contains("Missing threshold and requirement type for ROI merge")) shouldEqual 1 @@ -206,11 +201,12 @@ class TestParseDifferentTimepointsRoiMergeSectionOfImagingRoundsConfig extends test("With empty groups, no parse"): TraceIdDefinitionRulesSet.fromJson( - s"""{"discardRoisNotInGroupsOfInterest": true, "distanceThreshold": 5, "requirementType": "Conjunctive", "groups": []}""" + s"""{"discardRoisNotInGroupsOfInterest": true, "distanceThreshold": 5, "requirementType": "Conjunctive", "groups": {}}""" ) match { case Left(messages) => - messages.count(_.contains(s"Groups (from 'groups') is empty")) shouldEqual 1 - messages.length shouldEqual 1 + val exp = "Data for groups (from 'groups') is empty" + if messages.count(_.contains(exp)) === 1 then succeed + else fail(s"Expected a fail message with '$exp', but found non among these (${messages.length}): ${messages.mkString_("; ")}") case Right(_) => fail("Expected parse failure but got success") } @@ -219,14 +215,15 @@ class TestParseDifferentTimepointsRoiMergeSectionOfImagingRoundsConfig extends s"""{"discardRoisNotInGroupsOfInterest": true, "distanceThreshold": 5, "requirementType": "Conjunctive", "groups": null}""" ) match { case Left(messages) => - messages.count(_.contains(s"Can't parse groups (from 'groups') as array-like")) shouldEqual 1 - messages.length shouldEqual 1 + val exp = "Can't parse groups (from 'groups') as map-like" + if messages.count(_.contains(exp)) === 1 then succeed + else fail(s"Expected a fail message with '$exp', but found non among these (${messages.length}): ${messages.mkString_("; ")}") case Right(_) => fail("Expected parse failure but got success") } test("Without specification of discard policy, parse fails"): TraceIdDefinitionRulesSet.fromJson( - """{"distanceThreshold": 5, "requirementType": "Conjunctive", "groups": [[9, 10]]}""" + """{"distanceThreshold": 5, "requirementType": "Conjunctive", "groups": {"A": [9, 10]}}""" ) match { case Left(messages) => messages.count(_.contains("Missing key: 'discardRoisNotInGroupsOfInterest'")) shouldEqual 1 @@ -248,4 +245,55 @@ class TestParseDifferentTimepointsRoiMergeSectionOfImagingRoundsConfig extends test("No group may be a mapping."): pending + + private given arbGrouping(using Arbitrary[ImagingTimepoint]): Arbitrary[NonEmptyMap[String, AtLeast2[Set, ImagingTimepoint]]] = { + def genGroup(using arbTime: Arbitrary[ImagingTimepoint]): Gen[AtLeast2[Set, ImagingTimepoint]] = + Gen.choose(2, 10) + .flatMap(n => Gen.containerOfN[Set, ImagingTimepoint](n, arbTime.arbitrary)) + .suchThat(_.size >= 2) + .map{ g => + AtLeast2.either(g).fold( + msg => throw new Exception(s"Error building group for test: $msg"), + identity + ) + } + + def genGroupingOfOne(using Arbitrary[ImagingTimepoint]): Gen[NonEmptyList[AtLeast2[Set, ImagingTimepoint]]] = + genGroup.map(NonEmptyList.one) + + def genGroupingOfTwo: Gen[NonEmptyList[AtLeast2[Set, ImagingTimepoint]]] = + val indices = (0 to 10).map(ImagingTimepoint.unsafeLift) + for { + ts1 <- Gen.choose(2, 5) + .flatMap(n => Gen.pick(n, indices)) + .map(g => AtLeast2.unsafe(g.toSet)) + ts2 <- Gen.choose(2, indices.size - ts1.size) + .flatMap(n => Gen.pick(n, indices.toSet -- ts1.toSet)) + .map(g => AtLeast2.unsafe(g.toSet)) + } yield NonEmptyList.of(ts1, ts2) + + given Arbitrary[String] = // Generate nonempty, reasonable keys. + Gen.nonEmptyListOf( + Gen.oneOf( + Gen.alphaNumChar, + Gen.oneOf('-', '_', '.') + ) + ) + .map(_.mkString("")) + .toArbitrary + + def gen = for { + groups <- Gen.oneOf(genGroupingOfOne, genGroupingOfTwo) + keys <- Gen.listOfN(groups.length, Arbitrary.arbitrary[String]) + .suchThat(ks => ks.length === ks.toSet.size) + .map(_.toNel.getOrElse{ throw new Exception("Generated empty list from nonempty!") }) + } yield NonEmptyMap.of(keys.head -> groups.head, keys.tail.zip(groups.tail)*) + gen.toArbitrary + } + + private def makeGroupsJson: NonEmptyMap[String, AtLeast2[Set, ImagingTimepoint]] => String = + _.toSortedMap + .toList + .map{ (k, g) => s"\"$k\": [${g.toList.map(_.show_).mkString(", ")}]" } + .mkString(", ") end TestParseDifferentTimepointsRoiMergeSectionOfImagingRoundsConfig