Add v2.Problem.{get_output_parameters,get_x_nominal_dict}#447
Add v2.Problem.{get_output_parameters,get_x_nominal_dict}#447dweindl merged 3 commits intoPEtab-dev:mainfrom
v2.Problem.{get_output_parameters,get_x_nominal_dict}#447Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #447 +/- ##
==========================================
+ Coverage 74.79% 75.17% +0.37%
==========================================
Files 62 62
Lines 6845 6847 +2
Branches 1223 1223
==========================================
+ Hits 5120 5147 +27
+ Misses 1258 1232 -26
- Partials 467 468 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
* Move `get_output_parameters` from v2.lint to v2.Problem * Add `Problem.get_x_nominal_dict` * Test
dweindl
force-pushed
the
v2_get_output_parameters
branch
from
eb85bf5 to
6a656ce
Compare
petab/v2/core.py
Outdated
| ] | ||
|
|
||
| def get_output_parameters( | ||
| self, observables: bool = True, noise: bool = True |
There was a problem hiding this comment.
From the docstring, I think this would be more consistent. i.e. "collect free symbols from observable and noise formulas", rather than "collect free symbols from noise formulas and observables".
| self, observables: bool = True, noise: bool = True | |
| self, observable: bool = True, noise: bool = True |
There was a problem hiding this comment.
Agreed. Will differ from petab.v1.observables.get_output_parameters then, but fine with me.
petab/v2/core.py
Outdated
| for mapping in self.mappings: | ||
| if ( | ||
| mapping.petab_id == candidate | ||
| and mapping.model_id is not None | ||
| ): | ||
| if self.model.symbol_allowed_in_observable_formula( | ||
| mapping.model_id | ||
| ): | ||
| break | ||
| else: | ||
| # no mapping to a model entity, so it is an output parameter | ||
| output_parameters[candidate] = None |
There was a problem hiding this comment.
Fine as is but the spec currently states that mapping.model_id exists in the model, so the self.model.symbol_allowed_in_observable_formula check is redundant with linting that occurs elsewhere and can be assumed true here.
There was a problem hiding this comment.
the spec currently states that
mapping.model_idexists in the model
Hm, then the spec needs to be fixed. That would be incompatible with the intended use of the mapping table for general annotations.
There was a problem hiding this comment.
I think it's OK since it can be left empty. Here, the mapping.model_id is not None takes care of checking that it's non-empty (and thereby in the model according to the spec). Annotations would be in other columns as I recall.
A globally unique identifier defined in any model, or empty if the entity is not present in any model.
Co-authored-by: Dilan Pathirana <59329744+dilpath@users.noreply.github.com>
dweindl
force-pushed
the
v2_get_output_parameters
branch
from
b519edf to
30cce79
Compare
3 participants
get_output_parametersfrom v2.lint to v2.ProblemProblem.get_x_nominal_dict