Skip to content

fix: skip filename-based split detection for media files in get_data_… - #8257

Open
Work4itnow wants to merge 6 commits into
huggingface:mainfrom
Work4itnow:fix-imagefolder-split-detection
Open

fix: skip filename-based split detection for media files in get_data_…#8257
Work4itnow wants to merge 6 commits into
huggingface:mainfrom
Work4itnow:fix-imagefolder-split-detection

Conversation

@Work4itnow

Copy link
Copy Markdown

Fixes #7201

Problem

When a user has an image file named train.png in a flat folder,
the library mistakenly treats it as a split name instead of an image,
resulting in only that one file being loaded.

Fix

Modified _get_data_files_patterns in data_files.py to skip
filename-based split detection when all matched files are media files
(images, audio, video). This causes it to fall through to
DEFAULT_PATTERNS_ALL which correctly loads all files.

Test

Added test_data_files_with_image_named_after_split to verify
that all images are loaded when one is named after a split.

@ebarkhordar ebarkhordar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MEDIA_EXTENSIONS is missing a comma after ".tif", so Python concatenates that literal with the next one. The set ends up holding ".tif.mp3", and neither ".tif" nor ".mp3" is in it.

At this PR's head (3235cad), in a clean container:

>>> sorted(datasets.data_files.MEDIA_EXTENSIONS)
['.aac', '.avi', '.bmp', '.flac', '.gif', '.jpeg', '.jpg', '.m4a', '.mkv',
 '.mov', '.mp4', '.ogg', '.png', '.tif.mp3', '.tiff', '.wav', '.webm', '.webp']

So the original bug survives for those two extensions. Three files in a flat folder, one of them named train.<ext>, resolved with get_data_patterns() then DataFilesDict.from_patterns():

.png    ['pika.png', 'pika_pika.png', 'train.png']
.tiff   ['pika.tiff', 'pika_pika.tiff', 'train.tiff']
.wav    ['pika.wav', 'pika_pika.wav', 'train.wav']
.tif    ['train.tif']
.mp3    ['train.mp3']

On main (b305031) all five return only the train.* file, so the fix does work. It just misses .mp3, which is the extension an audio folder is most likely to use. The new test covers .png, so CI stays green either way.

@Work4itnow

Copy link
Copy Markdown
Author

Thank you for helping me catch it! I add the missing comma.

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.

load_dataset() of images from a single directory where train.png image exists

2 participants