Skip to content

Lazy loading with parametrized execution based on the different bool …#102

Open
Jeenashaji wants to merge 2 commits intoANCPLabOldenburg:mainfrom
Jeenashaji:main
Open

Lazy loading with parametrized execution based on the different bool …#102
Jeenashaji wants to merge 2 commits intoANCPLabOldenburg:mainfrom
Jeenashaji:main

Conversation

@Jeenashaji
Copy link
Contributor

…values for lazy loadfing using pytest

Lazy loading with parametrized execution based on the different bool values for lazy loadfing using pytest

@erdalkaraca erdalkaraca self-requested a review August 1, 2025 13:22
Copy link
Collaborator

@erdalkaraca erdalkaraca left a comment

Choose a reason for hiding this comment

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

You are properly applying fixtures to paramterize unit tests. We also need to parameterize the other existing unit tests in auto/* where datasets are loaded.

import shutil
import tempfile

from numpy.testing import tempdir
Copy link
Collaborator

Choose a reason for hiding this comment

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

seems to be a wrong/unused import, also clean up unused imports in general (tip: use pycharm's "Optimize Imports")

@@ -0,0 +1,245 @@
# Simplified parametrized test case using pytest fixtures
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove "Param_" prefix in file name



@pytest.fixture
def dataset_options(lazy_loading_option):
Copy link
Collaborator

Choose a reason for hiding this comment

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

merge this fixture with the load_dataset() fixture

def test_loading_completes_successfully(loaded_dataset, lazy_loading_option):
"""Test that both lazy and eager loading complete successfully."""
assert loaded_dataset is not None
assert hasattr(loaded_dataset, 'files')
Copy link
Collaborator

Choose a reason for hiding this comment

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

better to assert on the number of files and folders as these conditions will also pass on empty lists: how many files/folders do you expect to exist at the root of the dataset?

file_obj = loaded_dataset.get_file(file_name)
if file_obj: # File might not exist in test dataset
contents = file_obj.contents
assert contents is not None
Copy link
Collaborator

Choose a reason for hiding this comment

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

also (partially) check if the contents object contains fields that you expect should exist in the files - you may need to split into two unit tests, one for the tsv and the other for the json file

assert contents is not None


def test_content_caching_behavior(dataset_options, lazy_loading_option):
Copy link
Collaborator

Choose a reason for hiding this comment

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

if you need to have if/else here, it would make sense to split the test into 2 separate tests

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

Comments