Update/use ProblemConfig #353
Conversation
dweindl
commented
Dec 20, 2024
dweindl
commented
- add to_yaml
- refactor model, avoid extra elements in object tree
- use ProblemConfig in petab1to2
* add to_yaml * refactor model, avoid extra elements in object tree * use ProblemConfig in petab1to2
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #353 +/- ##
===========================================
- Coverage 74.89% 74.83% -0.06%
===========================================
Files 56 56
Lines 5604 5600 -4
Branches 982 982
===========================================
- Hits 4197 4191 -6
- Misses 1033 1035 +2
Partials 374 374 ☔ View full report in Codecov by Sentry. |
| condition_files: ListOfFiles = [] | ||
| observable_files: ListOfFiles = [] | ||
| visualization_files: ListOfFiles = [] | ||
| sbml_files: list[str | AnyUrl] = [] |
There was a problem hiding this comment.
This can be list[Path | AnyUrl], since pydantic has native support for pathlib.Path IIRC. But then you might need to make a custom path object that serializes to string https://github.com/dilpath/mkstd/blob/fb4ebfda629b0fb9eedb1def5ab05438ab85696e/mkstd/types/path.py
Fine as is, just noting in case it becomes useful later
| exclude=True, | ||
| ) | ||
| format_version: VersionNumber = "2.0.0" | ||
| format_version: str = "2.0.0" |
There was a problem hiding this comment.
str | int in v1 -- intentional difference?
There was a problem hiding this comment.
That's how it is in the current schema draft. Although not strictly enforced so far, I think it would make sense to always require major.minor. But that's probably a separate discussion.
There was a problem hiding this comment.
Since there is no benefit to keeping 1 as an int, could use str instead of str | int, to use pydantic to handle the type casting.
Requiring a full version string is fine for me, but then we need to provide schemas for each minor version. Makes sense to support minor updates to the format, so I'm also in favor of expecting a full X.X or X.X.X string. major.minor` as suggested is probably enough since we probably don't need to distinguish "minor" and "patch" updates
There was a problem hiding this comment.
Agreed, but I'll keep that for a future update.
There was a problem hiding this comment.
Added the format_version_tuple property for a normalized version number number.
3 participants
