Update petab.v2 #361

dweindl · 2025-03-17T15:45:50Z

Adapt petab.v2 to the updated specs draft.
Make the new pydantic objects the primary objects and generate the DataFrames only on demand.

Validation is still a bit rough. We'll need nicer error messages and pydantic settings are too lax for validating files.

codecov-commenter · 2025-03-17T15:51:41Z

Codecov Report

Attention: Patch coverage is 66.48148% with 181 lines in your changes missing coverage. Please review.

Project coverage is 74.55%. Comparing base (5b47448) to head (055dc2c).

Files with missing lines	Patch %	Lines
petab/v2/lint.py	62.31%	46 Missing and 32 partials ⚠️
petab/v2/problem.py	55.38%	52 Missing and 6 partials ⚠️
petab/v2/core.py	75.88%	30 Missing and 11 partials ⚠️
petab/v1/math/sympify.py	66.66%	2 Missing ⚠️
petab/v2/petab1to2.py	87.50%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #361      +/-   ##
===========================================
+ Coverage    74.44%   74.55%   +0.10%     
===========================================
  Files           57       57              
  Lines         5987     6075      +88     
  Branches      1039     1057      +18     
===========================================
+ Hits          4457     4529      +72     
- Misses        1133     1141       +8     
- Partials       397      405       +8

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

matthiaskoenig · 2025-03-21T14:50:54Z

@dweindl Is this usable already? I want to implement the timecourses in my tools but would need the updated petab library for that. Is this the right branch and working. If not what is still missing here?

dweindl · 2025-03-21T14:56:57Z

@dweindl Is this usable already? I want to implement the timecourses in my tools but would need the updated petab library for that. Is this the right branch and working. If not what is still missing here?

I'd suggest to wait another week. The whole petab.v2 should be considered experimental. There's a lot in motion.

I am currently aligning things with PEtab-dev/PEtab#581 and switching from DataFrame based functions to pydantic-objects-based ones for petab.v2.

matthiaskoenig · 2025-03-21T15:07:20Z

Perfect.

dilpath

Looks good so far!

petab/v1/math/sympify.py

petab/v2/core.py

dilpath · 2025-03-27T11:01:53Z

petab/v2/core.py

@@ -43,6 +49,43 @@
 ]


+def _is_finite_or_neg_inf(v: float, info: ValidationInfo) -> float:


Move these methods to the lint module? I guess they might be used there too, or is all validation intended to take place in the constructor of these pydantic objects?

That is still somewhat of an open question. I wouldn't want to duplicate the validation logic. However, the question is, which validations might have to be made optional at some point to work with future extensions. Probably too worry about that though.

So far, I would see everything that concerns the files and the presence of columns, as well as everything that cannot be validated inside a specific pydantic model to be in the lint module.

Another open question related to validation is whether it should still be possible to construct the pydantic objects from an invalid table.

No strong opinion but I guess that could be useful for e.g. loading partial problems into Paul's PEtab GUI

petab/v2/core.py

dilpath · 2025-03-27T11:12:58Z

petab/v2/core.py

+        if isinstance(v, int) or isinstance(v, float) and v.is_integer():
+            if v == 0:
+                return False
+            if v == 1:
+                return True
+
+        if isinstance(v, str):
+            if v == "0":
+                return False
+            if v == "1":
+                return True


Sufficient? Could also add True and False to remove the above code too...

Suggested change

if isinstance(v, int) or isinstance(v, float) and v.is_integer():

if v == 0:

return False

if v == 1:

return True

if isinstance(v, str):

if v == "0":

return False

if v == "1":

return True

if v in [0, 1, "0", "1"]:

return bool(int(v))

I think that should work. But meanwhile PEtab-dev/PEtab#614 happened. Needs updating.

Extra whitespace is not allowed, except for math expressions, right?

Extra whitespace is not allowed, except for math expressions, right?

Sounds fine to me. I don't see an issue either way, so whichever is more convenient

Same here. To be discussed at https://github.com/PEtab-dev/PEtab/.

dilpath · 2025-03-27T11:23:44Z

petab/v2/lint.py

+            if problem.model.has_entity_with_id(x)
+        }
+        if disallowed_in_condition:
+            is_or_are = "is" if len(disallowed_in_condition) == 1 else "are"


dilpath · 2025-03-27T11:31:51Z

tests/v2/test_conversion.py

    if problem_id == "Froehlich_CellSystems2018":
+        # this is mostly about 6M sympifications in the condition table
        pytest.skip("Too slow. Re-enable once we are faster.")


Not sure but maybe caching helps, e.g. https://github.com/AMICI-dev/AMICI/pull/1672/files

I had the feeling that caching is the problem here (inside sympy), but that needs further investigation.

Misc fixes

31da444

dweindl added 3 commits March 18, 2025 16:47

dfs->objs

5b87701

..

2e26c8d

lint

e5e1097

dweindl force-pushed the v2_misc branch from c50f4a8 to e5e1097 Compare March 21, 2025 13:32

dweindl force-pushed the v2_misc branch 2 times, most recently from e6db0d1 to 3b8fc54 Compare March 21, 2025 18:00

no conditionName, no operationType

7ec2a9b

dweindl force-pushed the v2_misc branch from 3b8fc54 to ba3953f Compare March 22, 2025 07:13

more linting

228a958

dweindl force-pushed the v2_misc branch from ba3953f to 228a958 Compare March 22, 2025 08:10

dweindl added 10 commits March 22, 2025 15:39

..

a3b1f66

..

f700fd9

..

1649080

Faster ConditionsTable.from_df

adbd5b6

allow extra

cd6cd3e

upconversion

7874190

more tests

433031e

more tests

ff093d0

todo

87b699e

cond. free syms

847d4a0

This was linked to issues Mar 24, 2025

Use proper object model for petab.v2 #337

Open

v2: handle optional condition table #291

Open

dweindl marked this pull request as ready for review March 25, 2025 08:20

dweindl requested a review from a team as a code owner March 25, 2025 08:20

..

1392112

dweindl mentioned this pull request Mar 25, 2025

lint: Don't fail on missing noiseFormula #367

Merged

more coherent naming

958d191

dilpath approved these changes Mar 27, 2025

View reviewed changes

estimate -> bool

055dc2c

dweindl merged commit 101c79c into PEtab-dev:develop Mar 27, 2025
7 checks passed

dweindl deleted the v2_misc branch March 27, 2025 13:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update petab.v2 #361

Update petab.v2 #361

dweindl commented Mar 17, 2025 •

edited

Loading

codecov-commenter commented Mar 17, 2025 •

edited

Loading

matthiaskoenig commented Mar 21, 2025

dweindl commented Mar 21, 2025

matthiaskoenig commented Mar 21, 2025

dilpath left a comment

dilpath Mar 27, 2025

dweindl Mar 27, 2025

dilpath Mar 27, 2025

dilpath Mar 27, 2025

dweindl Mar 27, 2025

dilpath Mar 27, 2025

dweindl Mar 27, 2025

dilpath Mar 27, 2025

dilpath Mar 27, 2025

dweindl Mar 27, 2025

		@@ -43,6 +49,43 @@
		]


		def _is_finite_or_neg_inf(v: float, info: ValidationInfo) -> float:

Update petab.v2 #361

Update petab.v2 #361

Conversation

dweindl commented Mar 17, 2025 • edited Loading

codecov-commenter commented Mar 17, 2025 • edited Loading

Codecov Report

matthiaskoenig commented Mar 21, 2025

dweindl commented Mar 21, 2025

matthiaskoenig commented Mar 21, 2025

dilpath left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dweindl commented Mar 17, 2025 •

edited

Loading

codecov-commenter commented Mar 17, 2025 •

edited

Loading