PEtab-dev / libpetab-python

Python package for working with PEtab files
https://libpetab-python.readthedocs.io
MIT License
14 stars 6 forks source link

Fix unreachable TypeError in petab.parameter_mapping.apply_overrides_… #60

Closed dweindl closed 3 years ago

dweindl commented 3 years ago

…for_observable (fixes #59)

Revert "More informative error messages in case of wrongly set observable and noise parameters (Closes PEtab-dev/PEtab#118) (PEtab-dev/PEtab#155)"

This reverts commit 8fab85e2b581ff501ee0a5595faa4d28543c0257.

codecov-io commented 3 years ago

Codecov Report

Merging #60 (72a65e6) into develop (d481eda) will increase coverage by 0.05%. The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #60      +/-   ##
===========================================
+ Coverage    78.79%   78.85%   +0.05%     
===========================================
  Files           24       24              
  Lines         2334     2331       -3     
  Branches       552      552              
===========================================
- Hits          1839     1838       -1     
+ Misses         361      359       -2     
  Partials       134      134              
Impacted Files Coverage Δ
petab/parameter_mapping.py 69.00% <100.00%> (+0.61%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d481eda...72a65e6. Read the comment docs.

FFroehlich commented 3 years ago

I think it would actually be nice to have an error message when mapping is not possible.

dweindl commented 3 years ago

I think it would actually be nice to have an error message when mapping is not possible.

Such a case would be invalid PEtab and should be reported by petablint. In the parameter mapping we widely assume we have valid PEtab files. If you have an example where this is not correctly detected, please share.

FFroehlich commented 3 years ago

I think it would actually be nice to have an error message when mapping is not possible.

Such a case would be invalid PEtab and should be reported by petablint. In the parameter mapping we widely assume we have valid PEtab files. If you have an example where this is not correctly detected, please share.

This is not detected for pysb-petab. Used observableParameters_$obs_id$ (extra s) or misspelled obs_id and both definitely did not yield the results I wanted.

dweindl commented 3 years ago

This is not detected for pysb-petab. Used observableParameters_$obs_id$ (extra s) or misspelled obs_id and both definitely did not yield the results I wanted.

Hm.... changing petab test case 0003:

diff --git a/petabtests/cases/pysb/0003/_observables.tsv b/petabtests/cases/pysb/0003/_observables.tsv
index dc2a127..43e4a45 100644
--- a/petabtests/cases/pysb/0003/_observables.tsv
+++ b/petabtests/cases/pysb/0003/_observables.tsv
@@ -1,2 +1,2 @@
 observableId   observableFormula       noiseFormula
-obs_a  observableParameter1_obs_a * A + observableParameter2_obs_a     0.5
+obs_a  observableParameter1_obs_a * A + observableParameters2_obs_a    0.5

gives me

petablint -y 0003/_0003.yaml 
SBML model not available. Skipping.
Mismatch of observable parameter overrides for obs_a (observableParameter1_obs_a * A + observableParameters2_obs_a)in:
observableId             obs_a
simulationConditionId       c0
time                         0
measurement                0.7
observableParameters     0.5;2
Name: 0, dtype: object
Expected 0 or 1 but got 2
Not OK

What am I doing differently?