Skip to content

Add predict kwargs in validation step #228

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

Merged
merged 9 commits into from
Jul 11, 2025
Merged

Add predict kwargs in validation step #228

merged 9 commits into from
Jul 11, 2025

Conversation

LeonStadelmann
Copy link
Collaborator

No description provided.

@LeonStadelmann LeonStadelmann requested a review from MUCDK May 1, 2025 17:00
@LeonStadelmann
Copy link
Collaborator Author

If tests are necessary, should they be added in test_cellflow or in test_trainer?

Copy link

codecov bot commented May 1, 2025

Codecov Report

Attention: Patch coverage is 0% with 7 lines in your changes missing coverage. Please review.

Project coverage is 0.00%. Comparing base (bdf8313) to head (2b323dd).
Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
src/cellflow/model/_cellflow.py 0.00% 5 Missing ⚠️
src/cellflow/training/_trainer.py 0.00% 2 Missing ⚠️

❌ Your project status has failed because the head coverage (0.00%) is below the target coverage (75.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main    #228   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files         38      38           
  Lines       2723    2727    +4     
=====================================
- Misses      2723    2727    +4     
Files with missing lines Coverage Δ
src/cellflow/training/_trainer.py 0.00% <0.00%> (ø)
src/cellflow/model/_cellflow.py 0.00% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@MUCDK
Copy link
Collaborator

MUCDK commented May 5, 2025

If tests are necessary, should they be added in test_cellflow or in test_trainer?

Let's please add a test checking for passing the number of steps of the solver, and make sure if it's 2 steps it's considerablyu faster (e.g. 1 sec) than passing 1e7 steps.

Let's add it to test_trainer

Copy link
Collaborator

@MUCDK MUCDK left a comment

Choose a reason for hiding this comment

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

Looks great, just add tests!

Copy link
Collaborator

@MUCDK MUCDK left a comment

Choose a reason for hiding this comment

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

Great, thanks!

@MUCDK
Copy link
Collaborator

MUCDK commented May 23, 2025

@LeonStadelmann thanks a lot, looks great!
Please resolve conflicts with main, then we can merge!

@LeonStadelmann LeonStadelmann requested a review from MUCDK July 10, 2025 13:00
Copy link
Collaborator

@MUCDK MUCDK left a comment

Choose a reason for hiding this comment

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

Thanks Leon!

@MUCDK
Copy link
Collaborator

MUCDK commented Jul 10, 2025

@selmanozleyen shall we remove the distinction of slow and non-slow tests now, wdyt?

@selmanozleyen
Copy link
Collaborator

selmanozleyen commented Jul 10, 2025

@MUCDK yeah maybe it makes more sense now. I can do that once this merges

@MUCDK MUCDK merged commit c574350 into main Jul 11, 2025
13 of 15 checks passed
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