Line 355 output message updated address misleading error message for empty detections in analyze_images() function#3157
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #3133 by improving an error message in the analyze_images() function. The original message incorrectly suggested that no images were found when the actual issue was that the analysis produced no detections (pose/keypoint predictions) from the images.
Key Changes
- Updated the error message from "Found no images in {images}" to "Images found in {images}, but no detections were made." to more accurately reflect that images exist but produced no detection results
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| if len(predictions) == 0: | ||
| print(f"Found no images in {images}") | ||
| print(f"Images found in {images}, but no detections were made.") |
There was a problem hiding this comment.
The message assumes images were found, but len(predictions) == 0 can occur in two scenarios: (1) no images matching the criteria were found (e.g., empty directory or wrong file extensions), or (2) images were found but produced no detections. The message is only accurate for scenario 2. Consider adding a check earlier in the function to distinguish between these cases and provide more accurate messages for each scenario.
| print(f"Images found in {images}, but no detections were made.") | |
| print( | |
| f"No detections were made; either no images were found in {images} or no objects were detected." | |
| ) |
There was a problem hiding this comment.
Agreed. Preferably only state that no detections were made, and confirm possible causes elsewhere.
|
Hi @Ronniekev, thanks for contributing! I think you need to revise your changes a bit: there is no ground for stating that the images were found, so better to avoid explicitly stating so. (see also CoPilot's comment). I'm posting my suggestions below, as I could not directly push my suggested changes. Could you maybe configure to grant access for reviewers to push changes directly? My suggestions below. Please check if you agree and if it works correctly for your use cases (e.g. empty folder, wrong extensions, or low-confidence detections). lines 354-356 if not predictions:
logging.info(f"No predictions made for images {images}.")
return {}lines 509-514 (after image_paths = parse_images_and_image_folders(images, image_suffixes)
if not image_paths:
logging.info(
f"No images found in {images}, with extension {image_suffixes}. "
"Skipping analysis."
)
return {}
pose_inputs = image_paths |
|
|
||
| if len(predictions) == 0: | ||
| print(f"Found no images in {images}") | ||
| print(f"Images found in {images}, but no detections were made.") |
There was a problem hiding this comment.
Agreed. Preferably only state that no detections were made, and confirm possible causes elsewhere.
…o images found and no predictions found).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
|
@Ronniekev thanks again for addressing this issue, merging these changes now. |
Addressing Issue #3133.
Misleading error message for empty detections updated to relay no detections found. Line 355 in analyze_images()