diff --git a/docs/sdk/tutorials/set_up_workflows.md b/docs/sdk/tutorials/set_up_workflows.md index b99ea762e..ac1a98fe9 100644 --- a/docs/sdk/tutorials/set_up_workflows.md +++ b/docs/sdk/tutorials/set_up_workflows.md @@ -90,6 +90,21 @@ kili.update_properties_in_project( You can manually select specific project assets to be used for computing consensus KPIs. +The method to use depends on the workflow version of your project. On multi-review +projects, use `update_asset_consensus`, one call per asset: + + +```python +for external_id in ["1.jpg", "2.jpg", "3.jpg"]: + kili.update_asset_consensus( + project_id=project_id, + external_id=external_id, + is_consensus=True, + ) +``` + +On projects still using workflow version 1, `update_asset_consensus` is not available. +Use `update_properties_in_assets` with `is_used_for_consensus_array` instead: ```python kili.update_properties_in_assets( @@ -99,14 +114,8 @@ kili.update_properties_in_assets( ) ``` - - - - [{'id': 'clnwvhvo00000gsvzinsato00'}, - {'id': 'clnwvhvo00001gsvzsiqcx5dc'}, - {'id': 'clnwvhvo00002gsvzzbjtyuif'}] - - +Using `is_used_for_consensus_array` on a multi-review project raises +`DeprecatedArgumentError`. For more information on consensus, refer to our [documentation](https://docs.kili-technology.com/docs/consensus-overview). diff --git a/recipes/set_up_workflows.ipynb b/recipes/set_up_workflows.ipynb index 8ec2f9165..187260b43 100644 --- a/recipes/set_up_workflows.ipynb +++ b/recipes/set_up_workflows.ipynb @@ -269,33 +269,44 @@ "source": [ "### Setting consensus for specific assets to compute consensus KPIs\n", "\n", - "You can manually select specific project assets to be used for computing consensus KPIs." + "You can manually select specific project assets to be used for computing consensus KPIs.\n", + "\n", + "The method to use depends on the workflow version of your project. On multi-review\n", + "projects, use `update_asset_consensus`, one call per asset:" ] }, { "cell_type": "code", "execution_count": null, "metadata": {}, - "outputs": [ - { - "data": { - "text/plain": [ - "[{'id': 'clnwvhvo00000gsvzinsato00'},\n", - " {'id': 'clnwvhvo00001gsvzsiqcx5dc'},\n", - " {'id': 'clnwvhvo00002gsvzzbjtyuif'}]" - ] - }, - "execution_count": null, - "metadata": {}, - "output_type": "execute_result" - } - ], + "outputs": [], "source": [ + "for external_id in [\"1.jpg\", \"2.jpg\", \"3.jpg\"]:\n", + " kili.update_asset_consensus(\n", + " project_id=project_id,\n", + " external_id=external_id,\n", + " is_consensus=True,\n", + " )" + ] + }, + { + "attachments": {}, + "cell_type": "markdown", + "metadata": {}, + "source": [ + "On projects still using workflow version 1, `update_asset_consensus` is not available.\n", + "Use `update_properties_in_assets` with `is_used_for_consensus_array` instead:\n", + "\n", + "```python\n", "kili.update_properties_in_assets(\n", " project_id=project_id,\n", " external_ids=[\"1.jpg\", \"2.jpg\", \"3.jpg\"],\n", " is_used_for_consensus_array=[True] * 3,\n", - ")" + ")\n", + "```\n", + "\n", + "Using `is_used_for_consensus_array` on a multi-review project raises\n", + "`DeprecatedArgumentError`." ] }, { diff --git a/src/kili/domain_api/assets.py b/src/kili/domain_api/assets.py index 6d2531835..8537da0b4 100644 --- a/src/kili/domain_api/assets.py +++ b/src/kili/domain_api/assets.py @@ -2369,6 +2369,9 @@ def update_consensus( ) -> bool: """Activate or deactivate consensus on an asset. + This method is not compatible with projects using workflow version 1. On those projects, + use `kili.update_properties_in_assets()` with `is_used_for_consensus_array` instead. + Args: project_id: The project ID. is_consensus: Whether to activate (True) or deactivate (False) consensus on the asset. diff --git a/src/kili/entrypoints/mutations/asset/__init__.py b/src/kili/entrypoints/mutations/asset/__init__.py index 64df33b4d..73a272ddd 100644 --- a/src/kili/entrypoints/mutations/asset/__init__.py +++ b/src/kili/entrypoints/mutations/asset/__init__.py @@ -27,12 +27,36 @@ GQL_UPDATE_PROPERTIES_IN_ASSETS, ) from kili.entrypoints.mutations.exceptions import MutationError -from kili.exceptions import MissingArgumentError +from kili.exceptions import DeprecatedArgumentError, GraphQLError, MissingArgumentError from kili.services.asset_import import import_assets from kili.services.asset_import_csv import get_text_assets_from_csv from kili.utils.assets import PageResolution from kili.utils.logcontext import for_all_methods, log_call +# The backend rejects `isUsedForConsensus` on multi-review projects (workflow version 2 and +# above). Its error message is the only place carrying this key: `extensions.code` is the generic +# `OPERATION_RESOLUTION_FAILURE` shared by nearly every domain error, so the bracketed token is +# the only usable discriminator. +IS_USED_FOR_CONSENSUS_DEPRECATED_KEY = "[isUsedForConsensusDeprecated]" +IS_USED_FOR_CONSENSUS_DEPRECATED_MESSAGE = ( + "`isUsedForConsensus` is deprecated in `update_properties_in_assets`." + " Use `update_asset_consensus` instead to manage consensus for this asset." +) + + +def _has_error_key(error: GraphQLError, key: str) -> bool: + """Tell whether any GraphQL error raised by the backend carries the given key. + + Every element is scanned because `GraphQLError` only ever renders the first one, and the + backend may report several errors for a single batch. + """ + errors = error.error if isinstance(error.error, list) else [error.error] + for item in errors: + message = item.get("message", "") if isinstance(item, dict) else str(item) + if key in message: + return True + return False + @for_all_methods(log_call, exclude=["__init__"]) class MutationsAsset(BaseOperationEntrypointMixin): @@ -317,6 +341,8 @@ def update_properties_in_assets( to each frame of the video. status_array: DEPRECATED and does not have any effect. is_used_for_consensus_array: Whether to use the asset to compute consensus kpis or not. + Not supported on multi-review projects, where it raises + `DeprecatedArgumentError`. Use `kili.update_asset_consensus()` for those projects. is_honeypot_array: Whether to use the asset for honeypot. project_id: The project ID. Only required if `external_ids` argument is provided. resolution_array: The resolution of each asset (for image and video assets). @@ -330,6 +356,13 @@ def update_properties_in_assets( Returns: A list of dictionaries with the asset ids. + Raises: + DeprecatedArgumentError: If `is_used_for_consensus_array` is used on a multi-review + project. Use `kili.update_asset_consensus()` for those projects. + MissingArgumentError: If both `asset_ids` and `external_ids` are provided, or if + neither of them is. + GraphQLError: If the backend refuses the update for any other reason. + Examples: >>> kili.update_properties_in_assets( asset_ids=["ckg22d81r0jrg0885unmuswj8", "ckg22d81s0jrh0885pdxfd03n"], @@ -417,12 +450,17 @@ def generate_variables(batch: dict) -> dict: "dataArray": data_array, } - results = mutate_from_paginated_call( - self, - properties_to_batch, - generate_variables, - GQL_UPDATE_PROPERTIES_IN_ASSETS, - ) + try: + results = mutate_from_paginated_call( + self, + properties_to_batch, + generate_variables, + GQL_UPDATE_PROPERTIES_IN_ASSETS, + ) + except GraphQLError as err: + if _has_error_key(err, IS_USED_FOR_CONSENSUS_DEPRECATED_KEY): + raise DeprecatedArgumentError(IS_USED_FOR_CONSENSUS_DEPRECATED_MESSAGE) from err + raise formated_results = [self.format_result("data", result, None) for result in results] return [item for batch_list in formated_results for item in batch_list] diff --git a/src/kili/exceptions.py b/src/kili/exceptions.py index 7a5f6b1ab..6f7468afc 100644 --- a/src/kili/exceptions.py +++ b/src/kili/exceptions.py @@ -65,3 +65,7 @@ class MissingArgumentError(ValueError): class IncompatibleArgumentsError(ValueError): """Raised when the user gave at least two incompatible arguments.""" + + +class DeprecatedArgumentError(ValueError): + """Raised when the user gave an argument that is no longer supported.""" diff --git a/src/kili/presentation/client/asset.py b/src/kili/presentation/client/asset.py index 1128c65c3..a75e5fb52 100644 --- a/src/kili/presentation/client/asset.py +++ b/src/kili/presentation/client/asset.py @@ -1,5 +1,6 @@ """Client presentation methods for assets.""" +# pylint: disable=too-many-lines import warnings from collections.abc import Generator, Iterable from typing import ( @@ -949,6 +950,9 @@ def update_asset_consensus( ) -> bool: """Activate or deactivate consensus on an asset. + This method is not compatible with projects using workflow version 1. On those projects, + use `kili.update_properties_in_assets()` with `is_used_for_consensus_array` instead. + Args: project_id: The project ID. is_consensus: Whether to activate (True) or deactivate (False) consensus on the asset. diff --git a/tests/integration/entrypoints/client/mutations/test_assets.py b/tests/integration/entrypoints/client/mutations/test_assets.py index 10ffdd06c..c93e20ff7 100644 --- a/tests/integration/entrypoints/client/mutations/test_assets.py +++ b/tests/integration/entrypoints/client/mutations/test_assets.py @@ -4,6 +4,7 @@ import pytest_mock from kili.entrypoints.mutations.asset import MutationsAsset, PageResolution +from kili.exceptions import DeprecatedArgumentError, GraphQLError @pytest.mark.parametrize( @@ -79,3 +80,83 @@ def test_given_asset_resolution_when_updating_resolution_then_it_works( "whereArray": [{"id": "asset_id_1"}], "dataArray": [{"resolution": {"width": 100, "height": 200}}], } + + +def _backend_error(message: str) -> GraphQLError: + """Build the error the graphql client raises when the backend refuses the mutation.""" + return GraphQLError( + error=[ + { + "message": message, + "extensions": { + "code": "OPERATION_RESOLUTION_FAILURE", + "context": {"projectID": "project_id"}, + }, + } + ] + ) + + +def test_given_multi_review_project_when_i_use_is_used_for_consensus_then_i_get_a_clear_error( + mocker: pytest_mock.MockerFixture, +): + # Given + kili = MutationsAsset() + kili.graphql_client = mocker.MagicMock() + kili.http_client = mocker.MagicMock() + kili.kili_api_gateway = mocker.MagicMock() + kili.graphql_client.execute.side_effect = _backend_error( + "[isUsedForConsensusDeprecated] `isUsedForConsensus` is deprecated in" + " `update_properties_in_assets`. Use `update_asset_consensus` instead to manage consensus" + " for this asset." + ) + + # When + with pytest.raises(DeprecatedArgumentError) as exc_info: + kili.update_properties_in_assets( + asset_ids=["asset_id_1"], is_used_for_consensus_array=[True] + ) + + # Then + assert str(exc_info.value) == ( + "`isUsedForConsensus` is deprecated in `update_properties_in_assets`." + " Use `update_asset_consensus` instead to manage consensus for this asset." + ) + assert isinstance(exc_info.value.__cause__, GraphQLError) + assert "[isUsedForConsensusDeprecated]" in str(exc_info.value.__cause__) + + +def test_given_workflow_v1_project_when_i_use_is_used_for_consensus_then_the_field_is_sent( + mocker: pytest_mock.MockerFixture, +): + # Given + kili = MutationsAsset() + kili.graphql_client = mocker.MagicMock() + kili.http_client = mocker.MagicMock() + kili.kili_api_gateway = mocker.MagicMock() + + # When + kili.update_properties_in_assets( + asset_ids=["asset_id_1", "asset_id_2"], is_used_for_consensus_array=[True, False] + ) + + # Then + assert kili.graphql_client.execute.call_args[0][1] == { + "whereArray": [{"id": "asset_id_1"}, {"id": "asset_id_2"}], + "dataArray": [{"isUsedForConsensus": True}, {"isUsedForConsensus": False}], + } + + +def test_given_an_unrelated_backend_error_when_i_update_properties_then_it_is_not_converted( + mocker: pytest_mock.MockerFixture, +): + # Given + kili = MutationsAsset() + kili.graphql_client = mocker.MagicMock() + kili.http_client = mocker.MagicMock() + kili.kili_api_gateway = mocker.MagicMock() + kili.graphql_client.execute.side_effect = _backend_error("[somethingElse] Another failure") + + # When / Then + with pytest.raises(GraphQLError): + kili.update_properties_in_assets(asset_ids=["asset_id_1"], priorities=[1])