Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle missing nominalValue in Problem.get_x_nominal #223

Merged
merged 1 commit into from
Jul 28, 2023

Conversation

dweindl
Copy link
Member

@dweindl dweindl commented Jul 28, 2023

It doesn't seem to be quite clear if the nominalValue column needs to be present if all parameters are estimated.
However, since it's allowed to leave nominalValue empty, which is treated as NaN, it seems to make sense to treat a missing nominalValue column as all-NaN.

See also ICB-DCM/pyPESTO#1104

@dweindl dweindl requested a review from a team as a code owner July 28, 2023 14:57
@codecov-commenter
Copy link

codecov-commenter commented Jul 28, 2023

Codecov Report

Merging #223 (9e52f08) into develop (d9b1e65) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop     #223      +/-   ##
===========================================
+ Coverage    76.33%   76.35%   +0.02%     
===========================================
  Files           34       34              
  Lines         3173     3176       +3     
  Branches       769      770       +1     
===========================================
+ Hits          2422     2425       +3     
  Misses         552      552              
  Partials       199      199              
Files Changed Coverage Δ
petab/problem.py 64.89% <100.00%> (+0.37%) ⬆️

@dweindl dweindl merged commit 3e42f8e into develop Jul 28, 2023
@dweindl dweindl deleted the fix_missing_nominalvalue branch July 28, 2023 16:35
dweindl added a commit that referenced this pull request Sep 17, 2023
It doesn't seem to be quite clear if the `nominalValue` column needs to be present if all parameters are estimated.
However, since it's allowed to leave `nominalValue` empty, which is treated as NaN, it seems to make sense to treat a missing  `nominalValue` column as all-NaN.

See also ICB-DCM/pyPESTO#1104
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants