From 877258f2765212ba271d5cfbf2b435a165a6990c Mon Sep 17 00:00:00 2001 From: davidnichols-ops Date: Mon, 29 Jun 2026 22:56:38 -0500 Subject: [PATCH 1/2] fix: return single_upload result from Project.upload() (#254) Project.upload() discarded the return value of single_upload(), returning None even on success. This made it impossible for callers to inspect the upload response (image id, timing, retry counts) without calling single_upload() directly. Now returns the single_upload() result dict for single-file uploads, and a list of such dicts for directory uploads. Existing callers that ignore the return value are unaffected. --- roboflow/core/project.py | 13 +++++++++++-- tests/test_project.py | 42 ++++++++++++++++++++++++++++++++++++++++ tests/test_queries.py | 3 ++- 3 files changed, 55 insertions(+), 3 deletions(-) diff --git a/roboflow/core/project.py b/roboflow/core/project.py index 05b1c773..2ce22a2f 100644 --- a/roboflow/core/project.py +++ b/roboflow/core/project.py @@ -409,6 +409,12 @@ def upload( metadata (dict, optional): custom key-value metadata to attach to the image. Example: {"camera_id": "cam001", "location": "warehouse"} + Returns: + For a single image: the dict returned by ``single_upload`` (keys: ``image``, + ``annotation``, ``upload_time``, ``annotation_time``, ``upload_retry_attempts``, + ``annotation_upload_retry_attempts``). For a directory: a list of such dicts, + one per successfully uploaded image. Skipped (non-image) files are excluded. + Example: >>> import roboflow @@ -445,7 +451,7 @@ def upload( ) ) - self.single_upload( + return self.single_upload( image_path=image_path, annotation_path=annotation_path, hosted_image=hosted_image, @@ -460,11 +466,12 @@ def upload( ) else: + results = [] images = os.listdir(image_path) for image in images: path = image_path + "/" + image if self.check_valid_image(path): - self.single_upload( + result = self.single_upload( image_path=path, annotation_path=annotation_path, hosted_image=hosted_image, @@ -477,10 +484,12 @@ def upload( metadata=metadata, **kwargs, ) + results.append(result) print("[ " + path + " ] was uploaded succesfully.") else: print("[ " + path + " ] was skipped.") continue + return results def upload_image( self, diff --git a/tests/test_project.py b/tests/test_project.py index 4c7d2a78..da3478c3 100644 --- a/tests/test_project.py +++ b/tests/test_project.py @@ -1,4 +1,5 @@ import json +import os from unittest.mock import patch import requests @@ -155,6 +156,47 @@ def test_upload_raises_upload_annotation_error(self): self.assertEqual(str(error.exception), "Image was already annotated.") + def test_upload_single_file_returns_result(self): + """upload() should return the single_upload result dict for a single file (#254).""" + image_id = "test-upload-id" + + responses.add( + responses.POST, + f"{API_URL}/dataset/{PROJECT_NAME}/upload?api_key={ROBOFLOW_API_KEY}&batch={DEFAULT_BATCH_NAME}", + json={"success": True, "id": image_id}, + status=200, + ) + + result = self.project.upload("tests/images/rabbit.JPG") + + self.assertIsInstance(result, dict) + self.assertEqual(result["image"]["id"], image_id) + self.assertIn("upload_time", result) + self.assertIn("upload_retry_attempts", result) + + def test_upload_directory_returns_list_of_results(self): + """upload() should return a list of single_upload results for a directory (#254).""" + test_dir = "tests/images" + # Determine how many valid images are in the directory so we can mock + # exactly that many upload responses. + valid_images = [f for f in os.listdir(test_dir) if self.project.check_valid_image(os.path.join(test_dir, f))] + + for i, _ in enumerate(valid_images): + responses.add( + responses.POST, + f"{API_URL}/dataset/{PROJECT_NAME}/upload?api_key={ROBOFLOW_API_KEY}&batch={DEFAULT_BATCH_NAME}", + json={"success": True, "id": f"img-{i}"}, + status=200, + ) + + result = self.project.upload(test_dir) + + self.assertIsInstance(result, list) + self.assertEqual(len(result), len(valid_images)) + for i, entry in enumerate(result): + self.assertIsInstance(entry, dict) + self.assertEqual(entry["image"]["id"], f"img-{i}") + def test_image_success(self): image_id = "test-image-id" expected_url = f"{API_URL}/{WORKSPACE_NAME}/{PROJECT_NAME}/images/{image_id}?api_key={ROBOFLOW_API_KEY}" diff --git a/tests/test_queries.py b/tests/test_queries.py index 93c29179..47e2b119 100644 --- a/tests/test_queries.py +++ b/tests/test_queries.py @@ -60,7 +60,8 @@ def test_project_methods(self): self.assertEqual(len(version_information), 2) self.assertIsNone(print_versions) self.assertTrue(all(map(lambda x: isinstance(x, Version), list_versions))) - self.assertIsNone(upload) + self.assertIsInstance(upload, dict) + self.assertEqual(upload["image"]["id"], "hbALkCFdNr9rssgOUXug") @ordered def test_version_fields(self): From 5698c093b43a7ae7f265d140be4f5c91f5118635 Mon Sep 17 00:00:00 2001 From: David Nichols Date: Mon, 3 Aug 2026 10:18:47 -0500 Subject: [PATCH 2/2] fix: return list from upload() in both single and directory cases Per review feedback on PR #502: wrap the single-file return in a list so upload() always returns list[dict] regardless of input type. - single file: return [single_upload_result] instead of single_upload_result - directory: unchanged (already returns list) - update docstring to reflect consistent list return type - update tests in test_project.py and test_queries.py accordingly --- roboflow/core/project.py | 37 ++++++++++++++++++++----------------- tests/test_project.py | 13 ++++++++----- tests/test_queries.py | 5 +++-- 3 files changed, 31 insertions(+), 24 deletions(-) diff --git a/roboflow/core/project.py b/roboflow/core/project.py index 2ce22a2f..ff55b1b3 100644 --- a/roboflow/core/project.py +++ b/roboflow/core/project.py @@ -410,10 +410,11 @@ def upload( Example: {"camera_id": "cam001", "location": "warehouse"} Returns: - For a single image: the dict returned by ``single_upload`` (keys: ``image``, - ``annotation``, ``upload_time``, ``annotation_time``, ``upload_retry_attempts``, - ``annotation_upload_retry_attempts``). For a directory: a list of such dicts, - one per successfully uploaded image. Skipped (non-image) files are excluded. + A list of result dicts (one per successfully uploaded image), regardless of + whether a single file or a directory was provided. Each dict is the return + value of ``single_upload`` (keys: ``image``, ``annotation``, ``upload_time``, + ``annotation_time``, ``upload_retry_attempts``, ``annotation_upload_retry_attempts``). + Skipped (non-image) files in directory mode are excluded from the list. Example: >>> import roboflow @@ -451,19 +452,21 @@ def upload( ) ) - return self.single_upload( - image_path=image_path, - annotation_path=annotation_path, - hosted_image=hosted_image, - image_id=image_id, - split=split, - num_retry_uploads=num_retry_uploads, - batch_name=batch_name, - tag_names=tag_names, - is_prediction=is_prediction, - metadata=metadata, - **kwargs, - ) + return [ + self.single_upload( + image_path=image_path, + annotation_path=annotation_path, + hosted_image=hosted_image, + image_id=image_id, + split=split, + num_retry_uploads=num_retry_uploads, + batch_name=batch_name, + tag_names=tag_names, + is_prediction=is_prediction, + metadata=metadata, + **kwargs, + ) + ] else: results = [] diff --git a/tests/test_project.py b/tests/test_project.py index da3478c3..c30b18cb 100644 --- a/tests/test_project.py +++ b/tests/test_project.py @@ -157,7 +157,7 @@ def test_upload_raises_upload_annotation_error(self): self.assertEqual(str(error.exception), "Image was already annotated.") def test_upload_single_file_returns_result(self): - """upload() should return the single_upload result dict for a single file (#254).""" + """upload() should return a list with the single_upload result dict for a single file (#254).""" image_id = "test-upload-id" responses.add( @@ -169,10 +169,13 @@ def test_upload_single_file_returns_result(self): result = self.project.upload("tests/images/rabbit.JPG") - self.assertIsInstance(result, dict) - self.assertEqual(result["image"]["id"], image_id) - self.assertIn("upload_time", result) - self.assertIn("upload_retry_attempts", result) + self.assertIsInstance(result, list) + self.assertEqual(len(result), 1) + entry = result[0] + self.assertIsInstance(entry, dict) + self.assertEqual(entry["image"]["id"], image_id) + self.assertIn("upload_time", entry) + self.assertIn("upload_retry_attempts", entry) def test_upload_directory_returns_list_of_results(self): """upload() should return a list of single_upload results for a directory (#254).""" diff --git a/tests/test_queries.py b/tests/test_queries.py index 47e2b119..267010ab 100644 --- a/tests/test_queries.py +++ b/tests/test_queries.py @@ -60,8 +60,9 @@ def test_project_methods(self): self.assertEqual(len(version_information), 2) self.assertIsNone(print_versions) self.assertTrue(all(map(lambda x: isinstance(x, Version), list_versions))) - self.assertIsInstance(upload, dict) - self.assertEqual(upload["image"]["id"], "hbALkCFdNr9rssgOUXug") + self.assertIsInstance(upload, list) + self.assertEqual(len(upload), 1) + self.assertEqual(upload[0]["image"]["id"], "hbALkCFdNr9rssgOUXug") @ordered def test_version_fields(self):