Skip to content

Bug: Duplicate loop in label_fps dict comprehension #159

Description

@mmcdermott

Description

In src/MEDS_tabular_automl/tabular_dataset.py:128-131:

label_fps = {
    shard: (Path(self.cfg.path.input_label_cache_dir) / self.split / shard).with_suffix(".parquet")
    for shard in self._data_shards
    for shard in self._data_shards  # <-- duplicate loop
}

The for shard in self._data_shards loop is repeated twice. This creates a nested iteration where the inner loop shadows the outer variable. While the result happens to be correct (the dictionary deduplicates keys), it performs len(shards)^2 iterations instead of len(shards).

Fix

Remove the duplicate line:

label_fps = {
    shard: (Path(self.cfg.path.input_label_cache_dir) / self.split / shard).with_suffix(".parquet")
    for shard in self._data_shards
}

Suggested test

A unit test that mocks _data_shards and verifies the resulting label_fps dict has exactly len(_data_shards) entries with the expected paths would catch a regression. Additionally, a linter rule for shadowed loop variables (e.g., pylint W0621) would flag this statically.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions