Skip to content

[MNT] Add missing assertions to run function tests - #1758

Closed
AtharvaManale wants to merge 3 commits into
openml:mainfrom
AtharvaManale:test_run_functions
Closed

AtharvaManale wants to merge 3 commits into
openml:mainfrom
AtharvaManale:test_run_functions

Conversation

@AtharvaManale

Copy link
Copy Markdown

Description

This PR adds missing assertions to the run function tests as part of #1646.

Changes

  • Added an assertion verifying that the task associated with the run uses the holdout estimation procedure.
  • Added assertions verifying that a run trace is present on both the original and downloaded run.
  • Added an equality check to verify that the trace is preserved when the run is downloaded.

Not addressed

The following existing TODOs are intentionally left unchanged because they require separate investigation or broader test changes:

  • The TODO concerning mocking initialize_model_from_trace() / server-side evaluation processing. The current test still uses the existing server-dependent flow.
  • Runtine check TODO waas ignored cause it was already validated by _check_fold_timing_evaluations function.

This keeps the PR focused on the assertions addressed here rather than mixing in unrelated test refactoring.

Testing

  • Ran the affected run-function tests locally.
  • Verified the added assertions against the OpenML test server.

Related to #1646.

@PGijsbers

Copy link
Copy Markdown
Collaborator

Hi, thanks for taking the time to contribute, but it seems like this is a duplicate of #1658. We'll move forward with that PR.

@PGijsbers PGijsbers closed this Oct 1, 2026
@AtharvaManale

Copy link
Copy Markdown
Author

@PGijsbers Thanks for the review, but I think it's not same as #1658, as I have implemented correct/detailed asserts for holdout task, trace_iterations of a run and also comparing the downloaded_run_trace with existing run.trace to check whether downloaded run carries the trace as objects.
I would appreciate if you review them too and let me know about some corrections.

Again thanks for the review @PGijsbers Sir.

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.

2 participants