Skip to content

[ML] Move the RDataLoader tests to pytest - #23819

Draft
siliataider wants to merge 8 commits into
root-project:masterfrom
siliataider:rdataloader-tests-pytest
Draft

siliataider wants to merge 8 commits into
root-project:masterfrom
siliataider:rdataloader-tests-pytest

Conversation

@siliataider

Copy link
Copy Markdown
Contributor

This Pull request:

Changes:

Moves ml_dataloader.py from unittest to pytest to make the tests shorter a,nd easier to read and to extend.
The test logic is unchanged.

  • Much shorter file: 8127 --> 4969 lines (-39%), with 104 tests in 74 test functions instead of 105
  • Fixtures for the input data: each test can directly ask for a common input built with the fixtures instead of writing its own
  • No more file cleanup code: each test runs in its own tmp_path, created and deleted by pytest
    • This removes all the try/except/finally blocks and the os.removes that existed only to delete temporary files
    • Tests can't clash on file names anymore
  • One test class for both lazy and eager loading: the copy-pasted lazy/eager classes are merged into TestDataLoaderSingleDataframe and TestDataLoaderMultipleDataframes, which runn all of their tests twice (for both loading modes) using a parametrized fixture
  • Skip instead of fail if package is missin: the PyTorch/TensorFlow/JAX tests are skipped when the framework isn't installed

Checklist:

  • tested changes locally

@siliataider siliataider self-assigned this Oct 23, 2026
@siliataider siliataider added the in:ML Everything under ROOT/ML label Oct 23, 2026
@siliataider
siliataider marked this pull request as draft October 23, 2026 15:54
@siliataider
siliataider force-pushed the rdataloader-tests-pytest branch from e650945 to 38118fb Compare October 23, 2026 16:09
@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Test Results

    24 files      24 suites   3d 22h 30m 29s ⏱️
 3 903 tests  3 903 ✅ 0 💤 0 ❌
83 563 runs  83 563 ✅ 0 💤 0 ❌

Results for commit f98b9b8.

♻️ This comment has been updated with latest results.

The JAX test needs it, like the PyTorch and TensorFlow tests need torch and tensorflow.

Assisted-by: Claude Opus 5.5
The test classes no longer derive from unittest.TestCase, their names get the "Test" prefix fot  pytest to collect, and their unittest assertions become the equivalent plain asserts.

Assisted-by: Claude Opus 5.5
Each test now runs in its own temporary directory which pytest creates and cleans up.
The next commits will use these.

Assisted-by: Claude Opus 5.5
This remooves the try/except/finally blocks in favour of using the fixtures defined previously.

Assisted-by: Claude Opus 5.5
…etrized test each

The eager and lazy versions of the tests on one and on multiple RDataFrames were near copies of each other
They are now merged into the sam etest class and use a parametrized fixture to switch between modes.
The tests that only existed for one of the modes still only run for it, they just don't take the fixture.

Assisted-by: Claude Opus 5.5
Each file is its own test, which only depends on the ML frameworks it uses.

Assisted-by: Claude Opus 5.5
@siliataider
siliataider force-pushed the rdataloader-tests-pytest branch from 38118fb to f98b9b8 Compare October 26, 2026 09:55

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI-Assisted in:ML Everything under ROOT/ML

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant