bambinos / bambi

BAyesian Model-Building Interface (Bambi) in Python.
https://bambinos.github.io/bambi/
MIT License
1.08k stars 124 forks source link

Improve tests #757

Closed tomicapretto closed 1 year ago

tomicapretto commented 1 year ago

As I wrote in #753, our tests were taking too much time to run. One of the reasons is that we had a test_predict.py file where we wanted to test whether we could compute predictions for a variety of models, but we ended up rebuilding and refitting many models already tested in test_built_models.py.

In this PR I'm moving and merging many tests from test_predict.py and test_built_model.py into test_models.py. There's a class for every model family. There we implement as many methods as needed. The goal is to avoid building and fitting the same model more than once.

I'm sure there's still room for improvement and reorganization. But I think this is now in a better state. And on top of that, I already invested a couple of days on this and I think it's more than enough :sweat_smile:.

As soon as the run finishes on my end I will add the time it takes now. Before these changes, running the tests took 1709.07 seconds on my end (28:29 minutes)

Edit After the changes it took 949.76 seconds (15:49)

codecov-commenter commented 1 year ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (c6e5dbb) 89.90% compared to head (e003f73) 89.60%. Report is 1 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #757 +/- ## ========================================== - Coverage 89.90% 89.60% -0.30% ========================================== Files 45 45 Lines 3713 3713 ========================================== - Hits 3338 3327 -11 - Misses 375 386 +11 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

GStechschulte commented 1 year ago

@tomicapretto I am pretty sure I prematurely merged this into main. Even though all the checks passed, this branch is behind main by 2 commits (per viewing this PR's branch). I think we need to revert and rebase first.

GStechschulte commented 12 months ago

@tomicapretto I am pretty sure I prematurely merged this into main. Even though all the checks passed, this branch is behind main by 2 commits (per viewing this PR's branch). I think we need to revert and rebase first.

Did we ever take care of this? 🙈