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

Introduce petab.v1 package #282

Merged
merged 7 commits into from
Jun 28, 2024
Merged

Conversation

dweindl
Copy link
Member

@dweindl dweindl commented Jun 27, 2024

As discussed in #271, we need to accommodate the changes related to the upcoming PEtab v2. The goal is to move PEtab 1.0 functionality to a petab.v1 subpackage and PEtab 2.0 functionality to a petab.v2 subpackage.

This PR moves (almost) all code from the petab package to a petab.v1 sub-package. Keeps all non-private objects importable from their previous location, but issues DeprecationWarnings.

From the next release on, all consumers should change all from petab[.$x] import $y to from petab.v1[.$x] import $y.

Fixes a couple of sphinx-issues that occurred on the way. Some obscure sphinx-failures remain, but they aren't critical.

Also adds some missing __all__s.

@codecov-commenter
Copy link

codecov-commenter commented Jun 27, 2024

Codecov Report

Attention: Patch coverage is 79.17143% with 729 lines in your changes missing coverage. Please review.

Project coverage is 75.32%. Comparing base (e8fdf3a) to head (58d0755).

Files Patch % Lines
petab/v1/math/_generated/PetabMathExprParser.py 73.02% 140 Missing and 34 partials ⚠️
petab/v1/lint.py 72.19% 67 Missing and 32 partials ⚠️
petab/v1/parameter_mapping.py 66.66% 54 Missing and 11 partials ⚠️
petab/v1/models/pysb_model.py 61.41% 44 Missing and 5 partials ⚠️
petab/v1/visualize/plotter.py 83.76% 27 Missing and 17 partials ⚠️
petab/v1/visualize/plotting.py 86.91% 21 Missing and 18 partials ⚠️
petab/v1/visualize/lint.py 56.79% 25 Missing and 10 partials ⚠️
petab/v1/parameters.py 85.63% 12 Missing and 15 partials ⚠️
petab/v1/models/model.py 69.84% 18 Missing and 1 partial ⚠️
petab/v1/core.py 89.24% 10 Missing and 7 partials ⚠️
... and 27 more
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #282      +/-   ##
===========================================
- Coverage    75.65%   75.32%   -0.33%     
===========================================
  Files           40       69      +29     
  Lines         4141     4256     +115     
  Branches       893      898       +5     
===========================================
+ Hits          3133     3206      +73     
- Misses         745      787      +42     
  Partials       263      263              

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

@dweindl dweindl marked this pull request as ready for review June 27, 2024 14:08
@dweindl dweindl requested review from dilpath, m-philipps and a team as code owners June 27, 2024 14:08
Copy link
Member

@dilpath dilpath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

I looked at most files and didn't see any issues 👍 I looked at petab/v1 less since I guess it's mostly copied.

Is it possible to include this in CI by running the tests with both like so?

import petab as petabv0
import petab.v1

petab = petabv0
# run tests
petab = petab.v1

Should the tests also be split into tests/v1 and tests/v2?

@dweindl
Copy link
Member Author

dweindl commented Jun 28, 2024

I looked at petab/v1 less since I guess it's mostly copied.

Yes.

Is it possible to include this in CI by running the tests with both like so?

import petab as petabv0
import petab.v1

petab = petabv0
# run tests
petab = petab.v1

For now, I would keep running tests through the deprecated functions. If they pass there, I don't have much doubt that they will work through petab.v1, as effectively everything was moved there. Once we remove the old modules, I would switch the tests.

Should the tests also be split into tests/v1 and tests/v2?

Yes, I think that makes sense. Will do that separately, though.

@dweindl dweindl merged commit 56b67ea into PEtab-dev:develop Jun 28, 2024
7 checks passed
@dweindl dweindl deleted the v1_subpackage_271 branch June 28, 2024 07:15
@dweindl dweindl linked an issue Jun 28, 2024 that may be closed by this pull request
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.

Library organization for petab v2
3 participants