diff --git a/CHANGELOG.md b/CHANGELOG.md index ff78cbbf..30e635d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,20 @@ All notable changes to this project will be documented in this file. +## Unreleased + +### Added + +- Move existing images into an annotation batch, by image IDs or by tag. + `Project.move_images_to_batch(batch_name=..., image_ids=[...])` (or + `tag=...`, or `batch_id=...` for an existing batch) returns + `{batchId, batchName, moved, skipped}`; `batch_name` reuses the batch with + that name or creates it. One call moves at most 1000 images, by IDs or by + tag; a larger tag is refused before anything moves. The CLI is + `roboflow annotation batch move -p (--batch-name N | --batch-id ID) + (--tag T | --image-id ID ...)`, with `--json` and the usual exit codes. + Needs the `POST /{workspace}/{project}/annotation-batches/move` API. + ## 1.7.2 ### Fixed diff --git a/CLI-COMMANDS.md b/CLI-COMMANDS.md index 11be906d..9f262ca3 100644 --- a/CLI-COMMANDS.md +++ b/CLI-COMMANDS.md @@ -268,6 +268,10 @@ roboflow annotation batch create -p my-project --source-batch-id \ --image-id --name "Review batch" roboflow annotation batch merge -p my-project --source-batch-id \ --target-batch-id --yes +# Regroup existing images into a batch, created by name if missing (max 1000 per call) +roboflow annotation batch move -p my-project --batch-name serial-0042 --tag serial-0042 +roboflow annotation batch move -p my-project --batch-id \ + --image-id --image-id roboflow annotation job admin-list -p my-project --limit 50 roboflow annotation job admin-get -p my-project diff --git a/roboflow/adapters/rfapi.py b/roboflow/adapters/rfapi.py index 35177ab3..321953dd 100644 --- a/roboflow/adapters/rfapi.py +++ b/roboflow/adapters/rfapi.py @@ -1147,6 +1147,31 @@ def merge_annotation_batches(api_key, workspace_url, project_url, *, source_batc return _annotation_administration_response(response) +# Per-call cap on images moved into a batch, for imageIds and for the images a tag resolves to. +MAX_MOVE_TO_BATCH_IMAGES = 1000 + + +def move_images_to_batch( + api_key, workspace_url, project_url, *, batch_name=None, batch_id=None, image_ids=None, tag=None +): + """POST /{ws}/{proj}/annotation-batches/move — move images, by IDs or tag, into a batch named or identified.""" + payload: Dict[str, Any] = {} + if image_ids is not None: + payload["imageIds"] = image_ids + if tag is not None: + payload["tag"] = tag + if batch_name is not None: + payload["batchName"] = batch_name + if batch_id is not None: + payload["batchId"] = batch_id + response = http.post( + f"{API_URL}/{workspace_url}/{project_url}/annotation-batches/move", + params={"api_key": api_key}, + json=payload, + ) + return _annotation_administration_response(response) + + def delete_annotation_batch(api_key, workspace_url, project_url, batch_id, *, permanent=False): """Delete a batch, retaining its images as unassigned unless permanent is true.""" response = http.delete( diff --git a/roboflow/cli/handlers/annotation.py b/roboflow/cli/handlers/annotation.py index f4da8b92..1f697797 100644 --- a/roboflow/cli/handlers/annotation.py +++ b/roboflow/cli/handlers/annotation.py @@ -122,6 +122,27 @@ def batch_merge( ) +@batch_app.command("move") +def batch_move( + ctx: typer.Context, + project: Annotated[str, typer.Option("-p", "--project", help="Project ID")], + batch_name: Annotated[ + Optional[str], typer.Option("--batch-name", help="Target batch name; created if no batch has it") + ] = None, + batch_id: Annotated[Optional[str], typer.Option("--batch-id", help="Target batch ID")] = None, + image_ids: Annotated[ + Optional[list[str]], typer.Option("--image-id", help="Image ID to move; repeat for multiple images") + ] = None, + tag: Annotated[ + Optional[str], typer.Option(help="Move every image in the project with this tag (at most 1000)") + ] = None, +) -> None: + """Move existing images, by ID or tag, into an annotation batch (at most 1000 per call).""" + _batch_move( + ctx_to_args(ctx, project=project, batch_name=batch_name, batch_id=batch_id, image_ids=image_ids, tag=tag) + ) + + @batch_app.command("delete") def batch_delete( ctx: typer.Context, @@ -513,6 +534,57 @@ def _batch_list(args: Any) -> None: output(args, batches, text=table) +def _batch_move(args: Any) -> None: + from roboflow.adapters import rfapi + from roboflow.cli._output import output, output_api_error, output_error + + if bool(args.image_ids) == (args.tag is not None): + output_error(args, "Choose the images with exactly one of --image-id or --tag.") + return + if (args.batch_name is None) == (args.batch_id is None): + output_error( + args, + "Choose the target with exactly one of --batch-name or --batch-id.", + hint="--batch-name creates the batch if it does not exist; --batch-id must name an existing batch.", + ) + return + if args.image_ids and len(args.image_ids) > rfapi.MAX_MOVE_TO_BATCH_IMAGES: + output_error( + args, + f"At most {rfapi.MAX_MOVE_TO_BATCH_IMAGES} images can be moved in one call.", + hint="Split the --image-id list across several calls.", + ) + return + context = _resolve_project_context(args) + if context is None: + return + try: + data = rfapi.move_images_to_batch( + *context, + batch_name=args.batch_name, + batch_id=args.batch_id, + image_ids=args.image_ids or None, + tag=args.tag, + ) + except rfapi.RoboflowError as exc: + over_tag_cap = args.tag is not None and str(rfapi.MAX_MOVE_TO_BATCH_IMAGES) in str(exc) + output_api_error( + args, + exc, + hint=( + f"A tag moves at most {rfapi.MAX_MOVE_TO_BATCH_IMAGES} images; move the rest with --image-id lists." + if over_tag_cap + else None + ), + not_found_hint="Check the project and --batch-id; 'roboflow annotation batch admin-list' lists batches.", + ) + return + text = f"Moved {data['moved']} image(s) to batch '{data['batchName']}' ({data['batchId']})." + if data["skipped"]: + text += f" Skipped {data['skipped']} already there or not movable." + output(args, data, text=text) + + def _job_list(args: Any) -> None: from roboflow.adapters import rfapi from roboflow.cli._output import output diff --git a/roboflow/core/project.py b/roboflow/core/project.py index 0f2003f3..d514bd1a 100644 --- a/roboflow/core/project.py +++ b/roboflow/core/project.py @@ -1096,6 +1096,61 @@ def merge_annotation_batches(self, source_batch_ids: List[str], target_batch_id: target_batch_id=target_batch_id, ) + def move_images_to_batch( + self, + batch_name: Optional[str] = None, + batch_id: Optional[str] = None, + image_ids: Optional[List[str]] = None, + tag: Optional[str] = None, + ) -> Dict: + """Move existing images into an annotation batch, from whichever batch holds them. + + Select the images with exactly one of ``image_ids`` or ``tag``, and the target with + exactly one of ``batch_name`` (created if no batch has that name) or ``batch_id``. + One call moves at most 1000 images: a longer ``image_ids`` list is refused here, and a + tag carried by more than 1000 images in the project is refused by the API before any + image moves. A call that would move nothing (every image already in the target + batch) is refused by the API. + + Args: + batch_name (str): name of the target batch; reused if it exists, created otherwise + batch_id (str): ID of an existing target batch + image_ids (list[str]): IDs of the images to move, at most 1000 + tag (str): move every image in the project carrying this tag, at most 1000 + + Returns: + dict: ``{"batchId", "batchName", "moved", "skipped"}``; ``moved`` and ``skipped`` + are counts, ``skipped`` being selected images left where they were + + Raises: + ValueError: if the exactly-one rules above are broken + RoboflowError: if the API rejects the request + + Example: + >>> project.move_images_to_batch(batch_name="serial-0042", tag="serial-0042") + """ + if (image_ids is None) == (tag is None): + raise ValueError("Pass exactly one of image_ids or tag.") + if image_ids is not None and not image_ids: + raise ValueError("image_ids cannot be empty.") + if image_ids is not None and len(image_ids) > rfapi.MAX_MOVE_TO_BATCH_IMAGES: + raise ValueError(f"At most {rfapi.MAX_MOVE_TO_BATCH_IMAGES} image_ids can be moved in one call.") + if tag is not None and not tag.strip(): + raise ValueError("tag cannot be empty.") + if (batch_name is None) == (batch_id is None): + raise ValueError("Pass exactly one of batch_name or batch_id.") + if not (batch_name or batch_id or "").strip(): + raise ValueError("batch_name or batch_id cannot be empty.") + return rfapi.move_images_to_batch( + self.__api_key, + self.__workspace, + self.__project_name, + batch_name=batch_name, + batch_id=batch_id, + image_ids=list(image_ids) if image_ids is not None else None, + tag=tag, + ) + def delete_annotation_batch(self, batch_id: str, permanent: bool = False) -> Dict: """Delete a batch, retaining its images as unassigned unless permanent is true.""" return rfapi.delete_annotation_batch( diff --git a/tests/adapters/test_annotation_administration.py b/tests/adapters/test_annotation_administration.py index 9359e37b..4b83038d 100644 --- a/tests/adapters/test_annotation_administration.py +++ b/tests/adapters/test_annotation_administration.py @@ -207,6 +207,27 @@ def test_delete_annotations_contract(self, mock_delete): rfapi.delete_annotation_job_annotations("key", "ws", "proj", "job-1") self.assertTrue(mock_delete.call_args.args[0].endswith("/annotation-jobs/job-1/annotations")) + @patch("roboflow.util.http.requests.post") + def test_move_images_to_batch_contract(self, mock_post): + mock_post.return_value = _response({"batchId": "b1", "batchName": "serial-42", "moved": 2, "skipped": 0}) + + result = rfapi.move_images_to_batch("key", "ws", "proj", batch_name="serial-42", image_ids=["i1", "i2"]) + self.assertTrue(mock_post.call_args.args[0].endswith("/ws/proj/annotation-batches/move")) + self.assertEqual(mock_post.call_args.kwargs["params"], {"api_key": "key"}) + self.assertEqual(mock_post.call_args.kwargs["json"], {"imageIds": ["i1", "i2"], "batchName": "serial-42"}) + self.assertEqual(result["moved"], 2) + + rfapi.move_images_to_batch("key", "ws", "proj", batch_id="b1", tag="serial-42") + self.assertEqual(mock_post.call_args.kwargs["json"], {"tag": "serial-42", "batchId": "b1"}) + + @patch("roboflow.util.http.requests.post") + def test_move_images_to_batch_error_raises_with_status(self, mock_post): + mock_post.return_value = _response({"error": {"message": "Batch not found"}}, status_code=404) + with self.assertRaises(rfapi.RoboflowError) as context: + rfapi.move_images_to_batch("key", "ws", "proj", batch_id="missing", tag="t") + self.assertEqual(context.exception.status_code, 404) + self.assertEqual(str(context.exception), "Batch not found") + @patch("roboflow.util.http.requests.get") def test_http_error_preserves_status_code(self, mock_get): mock_get.return_value = _response(status_code=403) diff --git a/tests/cli/test_annotation_handler.py b/tests/cli/test_annotation_handler.py index 569ae6aa..dc705260 100644 --- a/tests/cli/test_annotation_handler.py +++ b/tests/cli/test_annotation_handler.py @@ -39,7 +39,7 @@ def test_annotation_job_create(self): def test_full_annotation_administration_surface(self): commands = { - "batch": ["admin-list", "admin-get", "images", "create", "merge", "delete"], + "batch": ["admin-list", "admin-get", "images", "create", "merge", "move", "delete"], "job": [ "admin-list", "admin-get", @@ -518,5 +518,93 @@ def test_delete_annotations_executes_with_yes(self, _resolve, mock_api): mock_api.assert_called_once_with("key", "ws", "proj", "job-1") +class TestBatchMove(unittest.TestCase): + """annotation batch move""" + + _MOVE = "roboflow.adapters.rfapi.move_images_to_batch" + _RESULT = {"batchId": "b1", "batchName": "serial-42", "moved": 3, "skipped": 1} + + @patch(_MOVE, return_value=_RESULT) + @patch(_RESOLVE, return_value=("key", "ws", "proj")) + def test_move_by_tag_into_named_batch(self, _resolve, mock_api): + result = runner.invoke( + app, ["annotation", "batch", "move", "-p", "ws/proj", "--batch-name", "serial-42", "--tag", "serial-42"] + ) + self.assertEqual(result.exit_code, 0, result.output) + self.assertIn( + "Moved 3 image(s) to batch 'serial-42' (b1). Skipped 1 already there or not movable.", result.output + ) + mock_api.assert_called_once_with( + "key", "ws", "proj", batch_name="serial-42", batch_id=None, image_ids=None, tag="serial-42" + ) + + @patch(_MOVE, return_value=_RESULT) + @patch(_RESOLVE, return_value=("key", "ws", "proj")) + def test_move_by_ids_into_batch_id_json(self, _resolve, mock_api): + result = runner.invoke( + app, + ["--json", "annotation", "batch", "move", "-p", "ws/proj", "--batch-id", "b1"] + + ["--image-id", "i1", "--image-id", "i2"], + ) + self.assertEqual(result.exit_code, 0, result.output) + self.assertEqual(json.loads(result.output), self._RESULT) + mock_api.assert_called_once_with( + "key", "ws", "proj", batch_name=None, batch_id="b1", image_ids=["i1", "i2"], tag=None + ) + + @patch(_MOVE) + @patch(_RESOLVE, return_value=("key", "ws", "proj")) + def test_ambiguous_flags_exit_1_without_calling_api(self, _resolve, mock_api): + cases = [ + ["--batch-name", "b"], + ["--batch-name", "b", "--tag", "t", "--image-id", "i1"], + ["--tag", "t"], + ["--batch-name", "b", "--batch-id", "b1", "--tag", "t"], + ] + for flags in cases: + with self.subTest(flags=flags): + result = runner.invoke(app, ["--json", "annotation", "batch", "move", "-p", "ws/proj"] + flags) + self.assertEqual(result.exit_code, 1, result.output) + self.assertIn("exactly one", json.loads(result.output)["error"]["message"]) + mock_api.assert_not_called() + + @patch(_RESOLVE, return_value=("key", "ws", "proj")) + def test_api_errors_follow_exit_code_contract(self, _resolve): + from roboflow.adapters.rfapi import RoboflowError + + for status, code in [(401, 2), (404, 3), (400, 1)]: + with self.subTest(status=status), patch(self._MOVE, side_effect=RoboflowError("nope", status_code=status)): + result = runner.invoke( + app, ["--json", "annotation", "batch", "move", "-p", "ws/proj", "--batch-id", "b1", "--tag", "t"] + ) + self.assertEqual(result.exit_code, code, result.output) + self.assertEqual(json.loads(result.output)["error"]["message"], "nope") + + @patch(_MOVE) + @patch(_RESOLVE, return_value=("key", "ws", "proj")) + def test_more_than_1000_image_ids_exit_1_without_calling_api(self, _resolve, mock_api): + ids = [arg for i in range(1001) for arg in ("--image-id", f"i{i}")] + result = runner.invoke( + app, ["--json", "annotation", "batch", "move", "-p", "ws/proj", "--batch-id", "b1"] + ids + ) + self.assertEqual(result.exit_code, 1, result.output) + self.assertIn("At most 1000", json.loads(result.output)["error"]["message"]) + mock_api.assert_not_called() + + @patch(_RESOLVE, return_value=("key", "ws", "proj")) + def test_tag_over_cap_error_names_the_way_out(self, _resolve): + from roboflow.adapters.rfapi import RoboflowError + + message = 'Tag "big" matches more than 1000 images in this project; move them by imageIds in smaller selections' + with patch(self._MOVE, side_effect=RoboflowError(message, status_code=400)): + result = runner.invoke( + app, ["--json", "annotation", "batch", "move", "-p", "ws/proj", "--batch-name", "b", "--tag", "big"] + ) + self.assertEqual(result.exit_code, 1, result.output) + error = json.loads(result.output)["error"] + self.assertEqual(error["message"], message) + self.assertIn("--image-id", error["hint"]) + + if __name__ == "__main__": unittest.main() diff --git a/tests/test_project_annotation_administration.py b/tests/test_project_annotation_administration.py index 725573a4..29a8d500 100644 --- a/tests/test_project_annotation_administration.py +++ b/tests/test_project_annotation_administration.py @@ -39,6 +39,39 @@ def test_batch_wrappers_delegate_to_rfapi(self): self.assertEqual(getattr(self.project, method)(*args, **kwargs), {"ok": True}) mock.assert_called_once() + def test_move_images_to_batch_sends_one_selection_and_one_target(self): + response = {"batchId": "b1", "batchName": "serial-42", "moved": 2, "skipped": 0} + with patch("roboflow.adapters.rfapi.move_images_to_batch", return_value=response) as mock: + self.assertEqual( + self.project.move_images_to_batch(batch_name="serial-42", image_ids=["i1", "i2"]), response + ) + self.assertEqual( + mock.call_args.kwargs, + {"batch_name": "serial-42", "batch_id": None, "image_ids": ["i1", "i2"], "tag": None}, + ) + self.project.move_images_to_batch(batch_id="b1", tag="serial-42") + self.assertEqual( + mock.call_args.kwargs, + {"batch_name": None, "batch_id": "b1", "image_ids": None, "tag": "serial-42"}, + ) + + def test_move_images_to_batch_rejects_ambiguous_arguments_before_calling_api(self): + cases = [ + {"batch_name": "b"}, + {"batch_name": "b", "image_ids": ["i1"], "tag": "t"}, + {"batch_name": "b", "image_ids": []}, + {"batch_name": "b", "tag": " "}, + {"tag": "t"}, + {"batch_name": "b", "batch_id": "id", "tag": "t"}, + {"batch_name": "", "tag": "t"}, + {"batch_name": "b", "image_ids": [f"i{i}" for i in range(1001)]}, + ] + with patch("roboflow.adapters.rfapi.move_images_to_batch") as mock: + for kwargs in cases: + with self.subTest(kwargs=kwargs), self.assertRaises(ValueError): + self.project.move_images_to_batch(**kwargs) + mock.assert_not_called() + def test_job_wrappers_delegate_to_rfapi(self): cases = [ ("get_annotation_jobs", (), {}, "list_annotation_jobs"),