From ebb4f0582fc427758306b75c9593331272a814b7 Mon Sep 17 00:00:00 2001 From: Schmoho Date: Tue, 2 Aug 2022 13:16:18 +0200 Subject: [PATCH] issue #121 test #122 --- .../bigg/polishing/ReactionPolishing.java | 34 ++++++------ .../bigg/polishing/ReactionPolishingTest.java | 54 +++---------------- 2 files changed, 22 insertions(+), 66 deletions(-) diff --git a/src/main/java/edu/ucsd/sbrg/bigg/polishing/ReactionPolishing.java b/src/main/java/edu/ucsd/sbrg/bigg/polishing/ReactionPolishing.java index 24e78d3e..79ac0553 100644 --- a/src/main/java/edu/ucsd/sbrg/bigg/polishing/ReactionPolishing.java +++ b/src/main/java/edu/ucsd/sbrg/bigg/polishing/ReactionPolishing.java @@ -16,6 +16,7 @@ import java.util.ResourceBundle; import java.util.logging.Logger; import java.util.regex.Pattern; +import java.util.stream.Collectors; import static java.text.MessageFormat.format; @@ -159,8 +160,7 @@ private void setSBOTermFromPattern(BiGGId id) { * else {@link Optional#of}, where the wrapped string is the compartment code */ private Optional polish(ListOf speciesReferences, int defaultSBOterm) { - Optional compartmentId = Optional.empty(); - Model model = speciesReferences.getModel(); + // set defaults for (SpeciesReference sr : speciesReferences) { if (!sr.isSetSBOTerm() && !Parameters.get().omitGenericTerms()) { sr.setSBOTerm(defaultSBOterm); @@ -168,23 +168,21 @@ private Optional polish(ListOf speciesReferences, int if (!sr.isSetConstant()) { sr.setConstant(false); } - if (model == null) { - continue; - } - Species species = model.getSpecies(sr.getSpecies()); - if (species != null) { - // Assumed intention here, that conflicting compartment information cannot be resolved - if (!species.isSetCompartment() - || compartmentId.map(id -> !id.equals(species.getCompartment())).orElse(false)) { - return compartmentId; - } else { - compartmentId = Optional.of(species.getCompartment()); - } - } else { - logger.info(format(MESSAGES.getString("SPECIES_REFERENCE_INVALID"), sr.getSpecies())); - } } - return compartmentId; + // determine common compartment + Model model = speciesReferences.getModel(); + if (null != model) { + var modelSpecies = speciesReferences.stream() + .map(SpeciesReference::getSpeciesInstance) + .map(Optional::ofNullable) + .map(o -> o.map(Species::getCompartmentInstance)) + .flatMap(Optional::stream) + .map(Compartment::getId) + .collect(Collectors.toSet()); + + return modelSpecies.size() == 1 ? modelSpecies.stream().findFirst() : Optional.empty(); + } + return Optional.empty(); } diff --git a/src/test/java/edu/ucsd/sbrg/bigg/polishing/ReactionPolishingTest.java b/src/test/java/edu/ucsd/sbrg/bigg/polishing/ReactionPolishingTest.java index c939ba99..b2fde83b 100644 --- a/src/test/java/edu/ucsd/sbrg/bigg/polishing/ReactionPolishingTest.java +++ b/src/test/java/edu/ucsd/sbrg/bigg/polishing/ReactionPolishingTest.java @@ -157,51 +157,8 @@ public void conflictingProductsLeadToUnset() { assertEquals(null, r.getCompartmentInstance()); } - /** - * Note that this test is to be understood in tandem with the next one. - * Together they show that not only are inconsistent compartments in reaction - * products handled fundamentally different that in reactants, - * there also is an ordering dependency at play there. - *

- * I considers this behaviour a bug, the unit tests are here to document this - * and verify that the fix once implemented works. - */ @Test - public void orderingDependencyProductCompartmentsLeadToUnset1() { - var model = new Model(3, 2); - initParameters(); - var r = model.createReaction("some_reaction"); - var cytosol = model.createCompartment("c"); - var extracellular = model.createCompartment("e"); - - r.setCompartment(cytosol); - - var s1 = model.createSpecies("s1", cytosol); - r.createProduct(s1); - - var s2 = model.createSpecies("s2", extracellular); - r.createProduct(s2); - - assertEquals(cytosol, r.getCompartmentInstance()); - - new ReactionPolishing(r).polish(); - - assertEquals(2, r.getListOfProducts().size()); - assertEquals(Set.of(cytosol, extracellular), r.getListOfProducts().stream() - .map(SpeciesReference::getSpeciesInstance) - .map(Species::getCompartmentInstance) - .collect(Collectors.toSet())); - assertEquals(cytosol, r.getCompartmentInstance()); - } - - /** - * Please take note of the comment on the test above, as these only make - * sense in tandem. - *

- * tl;dr: this test documents a bug - */ - @Test - public void orderingDependencyProductCompartmentsLeadToUnset2() { + public void inconsistentProductCompartmentsLeadToUnset() { var model = new Model(3, 2); initParameters(); var r = model.createReaction("some_reaction"); @@ -296,10 +253,11 @@ public void defaultsAreSetOnSpeciesReferences() { } /** - * Note: this test documents a bug. + * Note: this test documents + * https://github.com/draeger-lab/ModelPolisher/issues/122 */ @Test - public void defaultsAreAbortedMidway() { + public void defaultsAreSetEvenIfCompartmentsAreInconsistent() { var model = new Model(3, 2); initParameters(); var r = model.createReaction("some_reaction"); @@ -327,10 +285,10 @@ public void defaultsAreAbortedMidway() { assertEquals(3, r.getListOfProducts().size()); assertEquals("SBO:0000011", product1.getSBOTermID()); assertEquals("SBO:0000011", product2.getSBOTermID()); - assertEquals("", product3.getSBOTermID()); + assertEquals("SBO:0000011", product3.getSBOTermID()); assertTrue(product1.isSetConstant()); assertTrue(product2.isSetConstant()); - assertFalse(product3.isSetConstant()); + assertTrue(product3.isSetConstant()); } /**