Skip to content

Commit 157003f

Browse files
davidnichols-opsdavidnichols-opsdigaobarbosa
authored
fix: return single_upload result from Project.upload() (#502)
* 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. * 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 --------- Co-authored-by: davidnichols-ops <your-email@example.com> Co-authored-by: Rodrigo Barbosa <rodrigo@roboflow.com>
1 parent 613e9f2 commit 157003f

3 files changed

Lines changed: 74 additions & 15 deletions

File tree

‎roboflow/core/project.py‎

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,13 @@ def upload(
409409
metadata (dict, optional): custom key-value metadata to attach to the image.
410410
Example: {"camera_id": "cam001", "location": "warehouse"}
411411
412+
Returns:
413+
A list of result dicts (one per successfully uploaded image), regardless of
414+
whether a single file or a directory was provided. Each dict is the return
415+
value of ``single_upload`` (keys: ``image``, ``annotation``, ``upload_time``,
416+
``annotation_time``, ``upload_retry_attempts``, ``annotation_upload_retry_attempts``).
417+
Skipped (non-image) files in directory mode are excluded from the list.
418+
412419
Example:
413420
>>> import roboflow
414421
@@ -445,26 +452,29 @@ def upload(
445452
)
446453
)
447454

448-
self.single_upload(
449-
image_path=image_path,
450-
annotation_path=annotation_path,
451-
hosted_image=hosted_image,
452-
image_id=image_id,
453-
split=split,
454-
num_retry_uploads=num_retry_uploads,
455-
batch_name=batch_name,
456-
tag_names=tag_names,
457-
is_prediction=is_prediction,
458-
metadata=metadata,
459-
**kwargs,
460-
)
455+
return [
456+
self.single_upload(
457+
image_path=image_path,
458+
annotation_path=annotation_path,
459+
hosted_image=hosted_image,
460+
image_id=image_id,
461+
split=split,
462+
num_retry_uploads=num_retry_uploads,
463+
batch_name=batch_name,
464+
tag_names=tag_names,
465+
is_prediction=is_prediction,
466+
metadata=metadata,
467+
**kwargs,
468+
)
469+
]
461470

462471
else:
472+
results = []
463473
images = os.listdir(image_path)
464474
for image in images:
465475
path = image_path + "/" + image
466476
if self.check_valid_image(path):
467-
self.single_upload(
477+
result = self.single_upload(
468478
image_path=path,
469479
annotation_path=annotation_path,
470480
hosted_image=hosted_image,
@@ -477,10 +487,12 @@ def upload(
477487
metadata=metadata,
478488
**kwargs,
479489
)
490+
results.append(result)
480491
print("[ " + path + " ] was uploaded succesfully.")
481492
else:
482493
print("[ " + path + " ] was skipped.")
483494
continue
495+
return results
484496

485497
def upload_image(
486498
self,

‎tests/test_project.py‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import json
2+
import os
23
from unittest.mock import patch
34

45
import requests
@@ -155,6 +156,50 @@ def test_upload_raises_upload_annotation_error(self):
155156

156157
self.assertEqual(str(error.exception), "Image was already annotated.")
157158

159+
def test_upload_single_file_returns_result(self):
160+
"""upload() should return a list with the single_upload result dict for a single file (#254)."""
161+
image_id = "test-upload-id"
162+
163+
responses.add(
164+
responses.POST,
165+
f"{API_URL}/dataset/{PROJECT_NAME}/upload?api_key={ROBOFLOW_API_KEY}&batch={DEFAULT_BATCH_NAME}",
166+
json={"success": True, "id": image_id},
167+
status=200,
168+
)
169+
170+
result = self.project.upload("tests/images/rabbit.JPG")
171+
172+
self.assertIsInstance(result, list)
173+
self.assertEqual(len(result), 1)
174+
entry = result[0]
175+
self.assertIsInstance(entry, dict)
176+
self.assertEqual(entry["image"]["id"], image_id)
177+
self.assertIn("upload_time", entry)
178+
self.assertIn("upload_retry_attempts", entry)
179+
180+
def test_upload_directory_returns_list_of_results(self):
181+
"""upload() should return a list of single_upload results for a directory (#254)."""
182+
test_dir = "tests/images"
183+
# Determine how many valid images are in the directory so we can mock
184+
# exactly that many upload responses.
185+
valid_images = [f for f in os.listdir(test_dir) if self.project.check_valid_image(os.path.join(test_dir, f))]
186+
187+
for i, _ in enumerate(valid_images):
188+
responses.add(
189+
responses.POST,
190+
f"{API_URL}/dataset/{PROJECT_NAME}/upload?api_key={ROBOFLOW_API_KEY}&batch={DEFAULT_BATCH_NAME}",
191+
json={"success": True, "id": f"img-{i}"},
192+
status=200,
193+
)
194+
195+
result = self.project.upload(test_dir)
196+
197+
self.assertIsInstance(result, list)
198+
self.assertEqual(len(result), len(valid_images))
199+
for i, entry in enumerate(result):
200+
self.assertIsInstance(entry, dict)
201+
self.assertEqual(entry["image"]["id"], f"img-{i}")
202+
158203
def test_image_success(self):
159204
image_id = "test-image-id"
160205
expected_url = f"{API_URL}/{WORKSPACE_NAME}/{PROJECT_NAME}/images/{image_id}?api_key={ROBOFLOW_API_KEY}"

‎tests/test_queries.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,9 @@ def test_project_methods(self):
6060
self.assertEqual(len(version_information), 2)
6161
self.assertIsNone(print_versions)
6262
self.assertTrue(all(map(lambda x: isinstance(x, Version), list_versions)))
63-
self.assertIsNone(upload)
63+
self.assertIsInstance(upload, list)
64+
self.assertEqual(len(upload), 1)
65+
self.assertEqual(upload[0]["image"]["id"], "hbALkCFdNr9rssgOUXug")
6466

6567
@ordered
6668
def test_version_fields(self):

0 commit comments

Comments
 (0)