feat: support tag_depth > 1 in add_images_from_path - #1775
feat: support tag_depth > 1 in add_images_from_path#1775lorinczszabolcs wants to merge 4 commits into
Conversation
Generalize tag_samples_by_directory to tag samples by the first N directory levels below the loaded path (previously only tag_depth=1 was supported) and raise ValueError on a negative tag_depth. Update the docstring, docs and changelog, and add tests for tag_depth=2 and the negative case.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesImage directory tagging
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lightly_studio/src/lightly_studio/core/image/image_dataset.py`:
- Around line 153-155: Validate negative tag_depth in the entry-point method
before loading images or performing filesystem/database work, alongside
validate_limit(). Keep the existing tag_samples_by_directory() validation so
direct callers remain protected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fd3dfc4b-496a-4e0d-ad31-fcb2f5d4aabe
📒 Files selected for processing (6)
CHANGELOG.mdlightly_studio/docs/docs/concepts_and_tools/tags.mdlightly_studio/docs/docs/dataset_setup/image_dataset.mdlightly_studio/src/lightly_studio/core/image/add_images.pylightly_studio/src/lightly_studio/core/image/image_dataset.pylightly_studio/tests/core/image/test_add_images.py
Negative tag_depth was only rejected inside tag_samples_by_directory, which runs only when new samples are created, so an invalid depth was silently accepted for empty or all-existing loads. Validate it in add_images_from_path alongside validate_limit, before any filesystem or database work, and keep the helper check for direct callers. Add a test.
|
@MalteEbner this extends the tag_depth support you reviewed in #84 to tag_depth > 1. Tagging in case the PR didn't reach you yet. |
What has changed and why?
add_images_from_pathonly supportedtag_depth=1(added in #84). This generalizes it totag_depth=N: each image is tagged with its firstNfolder levels below the loaded path (fewer if nested less deeply), so it can get several tags.tag_depth=0still skips tagging; negative values now raiseValueError. Docstrings, docs and changelog updated. No new dependencies.Requested in comment, which also suggested extending
tag_depthto the YOLO/COCO loaders, those are left for a separate PR.How has it been tested?
New unit tests in
tests/core/image/test_add_images.pyfortag_depth=2and the negative case; existingtag_depth=0/1and file-URL tests still pass.Did you update CHANGELOG.md?
Summary by CodeRabbit
tag_depth > 1).tag_depth=0disables folder-based tagging; tags can be derived from the firstNdirectory levels beneath the selected path.tag_depthbehavior and nested-folder examples.tag_depthand validatingtag_depth=2.