From 715e9adf591a36be598d4cbe59588d7fcd93b502 Mon Sep 17 00:00:00 2001 From: Roman Donchenko Date: Mon, 10 Jul 2023 19:20:20 +0300 Subject: [PATCH 1/2] Allow attribute default/permitted values to be blank This is de facto already allowed when CVAT is accessed via the UI/API, even though the model/serializer configuration seem to prohibit it. That's because validation for attribute properties is effectively disabled on the server due to a bug. However, the SDK checks the schema, and thus doesn't allow such values. I think blank default values make sense (particularly for "text" type attributes), so I updated the code to allow them. Blank permitted values don't make quite as much sense, but I had to allow them too, because the UI always submits the default value as the first permitted value (even for freeform text attributes). This will let us fix the broken validation on the server side (which I'm planning to do soon) without removing the ability to set blank default attribute values via the UI. (FWIW, I don't think that the UI should add a `values` property when serializing freeform text attributes at all, but it would take a more substantial change to fix that, which I don't have time for right now.) --- ...er_attributespec_default_value_and_more.py | 22 +++++++++++++++++ cvat/apps/engine/models.py | 4 ++-- cvat/apps/engine/serializers.py | 2 +- cvat/schema.yml | 4 ---- tests/python/sdk/test_projects.py | 24 +++++++++++++++++++ 5 files changed, 49 insertions(+), 7 deletions(-) create mode 100644 cvat/apps/engine/migrations/0073_alter_attributespec_default_value_and_more.py diff --git a/cvat/apps/engine/migrations/0073_alter_attributespec_default_value_and_more.py b/cvat/apps/engine/migrations/0073_alter_attributespec_default_value_and_more.py new file mode 100644 index 000000000000..7bfe361c875b --- /dev/null +++ b/cvat/apps/engine/migrations/0073_alter_attributespec_default_value_and_more.py @@ -0,0 +1,22 @@ +# Generated by Django 4.2.1 on 2023-07-10 15:57 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("engine", "0072_alter_issue_updated_date"), + ] + + operations = [ + migrations.AlterField( + model_name="attributespec", + name="default_value", + field=models.CharField(blank=True, max_length=128), + ), + migrations.AlterField( + model_name="attributespec", + name="values", + field=models.CharField(blank=True, max_length=4096), + ), + ] diff --git a/cvat/apps/engine/models.py b/cvat/apps/engine/models.py index 5e9f6781eb2c..8b1ee700271f 100644 --- a/cvat/apps/engine/models.py +++ b/cvat/apps/engine/models.py @@ -881,8 +881,8 @@ class AttributeSpec(models.Model): mutable = models.BooleanField() input_type = models.CharField(max_length=16, choices=AttributeType.choices()) - default_value = models.CharField(max_length=128) - values = models.CharField(max_length=4096) + default_value = models.CharField(blank=True, max_length=128) + values = models.CharField(blank=True, max_length=4096) class Meta: default_permissions = () diff --git a/cvat/apps/engine/serializers.py b/cvat/apps/engine/serializers.py index a92df38a0a22..f3303ea0ab03 100644 --- a/cvat/apps/engine/serializers.py +++ b/cvat/apps/engine/serializers.py @@ -214,7 +214,7 @@ class Meta: class AttributeSerializer(serializers.ModelSerializer): values = serializers.ListField(allow_empty=True, - child=serializers.CharField(max_length=200), + child=serializers.CharField(allow_blank=True, max_length=200), ) class Meta: diff --git a/cvat/schema.yml b/cvat/schema.yml index 47a68af2e933..cbb4310c538e 100644 --- a/cvat/schema.yml +++ b/cvat/schema.yml @@ -5993,7 +5993,6 @@ components: type: string maxLength: 200 required: - - default_value - input_type - mutable - name @@ -6011,16 +6010,13 @@ components: $ref: '#/components/schemas/InputTypeEnum' default_value: type: string - minLength: 1 maxLength: 128 values: type: array items: type: string - minLength: 1 maxLength: 200 required: - - default_value - input_type - mutable - name diff --git a/tests/python/sdk/test_projects.py b/tests/python/sdk/test_projects.py index 74ecf5e69084..852a286fc288 100644 --- a/tests/python/sdk/test_projects.py +++ b/tests/python/sdk/test_projects.py @@ -115,6 +115,30 @@ def test_can_create_empty_project(self): assert project.id != 0 assert project.name == "test project" + def test_can_create_project_with_attribute_with_blank_default(self): + project = self.client.projects.create( + spec=models.ProjectWriteRequest( + name="test project", + labels=[ + models.PatchedLabelRequest( + name="text", + attributes=[ + models.AttributeRequest( + name="text", + mutable=True, + input_type=models.InputTypeEnum("text"), + values=[], + default_value="", + ) + ], + ) + ], + ) + ) + + labels = project.get_labels() + assert labels[0].attributes[0].default_value == "" + def test_can_create_project_from_dataset(self, fxt_coco_dataset: Path): pbar_out = io.StringIO() pbar = make_pbar(file=pbar_out) From 5e29a990a5f6a300f415ad96b201dcf532ce3296 Mon Sep 17 00:00:00 2001 From: Roman Donchenko Date: Tue, 11 Jul 2023 13:09:52 +0300 Subject: [PATCH 2/2] Update changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8091b53ef437..7800f0fc6bb7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - TDB ### Fixed -- TDB +- \[SDK\] Ability to create attributes with blank default values + () ### Security - TDB