fix: fix ImageDataset axis order and add tests#105
Merged
Conversation
ganow
commented
Apr 15, 2025
Coverage Report
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Add tests/dl/torch/test_dataset.py with 9 test cases covering CHW axis order, per-channel values, DataLoader integration, value normalization, length, explicit stimulus ordering, and auto-detection via Path.stem. Also sort auto-detected stimulus names for deterministic ordering, and remove the empty TestImageDataset stub from test_torch.py. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
KenyaOtsuka
reviewed
May 13, 2026
KenyaOtsuka
left a comment
There was a problem hiding this comment.
Thanks for the fix. I left a few minor comments.
Optionally, since this changes the behavior of ImageDataset from HWC to CHW, it might be helpful to document that ImageDataset now returns images in CHW format.
| from pathlib import Path | ||
|
|
||
| import numpy as np | ||
| import torch |
There was a problem hiding this comment.
torch seems to be unused in this file. Could you remove it?
Contributor
Author
There was a problem hiding this comment.
that's true. thank you for mentioning it
| root = Path(self.tmpdir.name) | ||
| _save_image(root / "a.jpg", r=200, g=100, b=50) | ||
| _save_image(root / "b.jpg", r=10, g=20, b=30) | ||
| _save_image(root / "c.jpg", r=0, g=128, b=255) |
There was a problem hiding this comment.
For tests that check exact pixel values, it may be better to use a lossless format such as PNG.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
ImageDatasethad three bugs, with no tests to catch them:Wrong axis order in returned image array
__getitem__returned images in HWC format(H, W, C), but PyTorch'sDataLoaderand most deep learning models expect CHW format(C, H, W). This caused shape mismatches when feeding images into networks.Incorrect auto-detection of stimulus names
When
stimulus_names=None, file stems were extracted using a custom_removesuffixhelper instead ofPath.stem. This was unnecessarily verbose and fragile.Non-deterministic order of auto-detected stimulus names
When
stimulus_names=None,Path.glob()was used without sorting, so the order of stimulus names was filesystem-dependent and not reproducible.Fix
.transpose(2, 0, 1)to convert image arrays from HWC to CHW before returning._removesuffix-based list comprehension withpath.stem.sorted()to ensure auto-detected stimulus names are always in alphabetical order.Tests
Added
tests/dl/torch/test_dataset.pywith 9 test cases:test_getitem_returns_chw_shape— verifies CHW axis order using a non-square image (H≠W) to fully discriminate every axistest_getitem_preserves_channels— verifies per-channel values are correctly mapped after transposetest_dataloader_integration_batch_shape— end-to-end check viaDataLoader(the original failure path)test_value_range_normalized_to_unit_interval— verifies pixel values are in[0, 1]test_len_matches_stimulus_names— verifies__len__test_explicit_stimulus_names_respected— verifies explicitstimulus_namesare used as-istest_explicit_stimulus_names_preserve_input_order— verifies input order is preserved whenstimulus_namesis giventest_auto_detected_stimulus_names_use_stem— verifies file stems are used whenstimulus_names=Nonetest_auto_detected_stimulus_names_are_sorted— verifies alphabetical ordering of auto-detected namesAlso removed the empty
TestImageDatasetstub fromtest_torch.py.