EnzymeML / PyEnzyme

🧬 - Data management and modeling framework based on EnzymeML.
BSD 2-Clause "Simplified" License
21 stars 9 forks source link

Bug in `EnzymeMLDocument.__updateModelParameters` #10

Closed jmrohwer closed 3 years ago

jmrohwer commented 3 years ago

The __convertUnit() was calling the unit ID, but it should be calling the unit string. The for loop should iterate over the dictionary items, not only the keys. The following diff fixes it.

--- a/pyenzyme/enzymeml/core/enzymemldocument.py
+++ b/pyenzyme/enzymeml/core/enzymemldocument.py
@@ -881,10 +881,10 @@ class EnzymeMLDocument(object):
         return reactionID

     def __updateModelParameters(self, model):
-        for name, (value, unit) in model.getParameters():
+        for name, (value, unit) in model.getParameters().items():
             model.getParameters()[name] = (
                 value,
-                self.__convertUnit(unit)
+                self.__convertUnit(self.getUnitDef(unit).getName())
             )
JR-1991 commented 3 years ago

Thanks Johann! Closer inspection showed, that the updateModelParameters method is redundant, since unit conversion already happens at the level of the KineticModel initialization. Thus, a second conversion makes no sense. This was fixed in commit 30e1b77