fix: allow visible_labels=None & disallow fields/questions with the same name

CHANGELOG.md

@@ -37,6 +37,13 @@ These are the section headers that we use:
 - Database setup for unit tests. Now the unit tests use a different database than the one used by the local Argilla server (Closes [#2987]).
 - Updated `alembic` setup to be able to autogenerate revision/migration scripts using SQLAlchemy metadata from Argilla server models ([#3044])
 
+### Fixed
+
+- Disallow `fields` and `questions` in `FeedbackDataset` with the same name ([#3126]).
+- Keep `visible_labels=None` if `None` is specified in `LabelQuestion` or `MultiLabelQuestion`, otherwise, use default 20 ([#3126]).
+
+[#3126]: https://github.com/argilla-io/argilla/pull/3126
+
 ## [1.8.0](https://github.com/argilla-io/argilla/compare/v1.7.0...v1.8.0)
 
 ### Added

src/argilla/client/feedback/dataset.py

@@ -207,9 +207,13 @@ def __init__(
         if not isinstance(fields, list):
             raise TypeError(f"Expected `fields` to be a list, got {type(fields)} instead.")
         any_required = False
+        unique_names = set()
         for field in fields:
             if not isinstance(field, FieldSchema):
                 raise TypeError(f"Expected `fields` to be a list of `FieldSchema`, got {type(field)} instead.")
+            if field.name in unique_names:
+                raise ValueError(f"Expected `fields` to have unique names, got {field.name} twice instead.")
+            unique_names.add(field.name)
             if not any_required and field.required:
                 any_required = True
         if not any_required:
@@ -220,13 +224,17 @@ def __init__(
         if not isinstance(questions, list):
             raise TypeError(f"Expected `questions` to be a list, got {type(questions)} instead.")
         any_required = False
+        unique_names = set()
         for question in questions:
             if not isinstance(question, (TextQuestion, RatingQuestion, LabelQuestion, MultiLabelQuestion)):
                 raise TypeError(
                     "Expected `questions` to be a list of `TextQuestion`, `RatingQuestion`,"
                     " `LabelQuestion`, and/or `MultiLabelQuestion` got a"
                     f" question in the list with type {type(question)} instead."
                 )
+            if question.name in unique_names:
+                raise ValueError(f"Expected `questions` to have unique names, got {question.name} twice instead.")
+            unique_names.add(question.name)
             if not any_required and question.required:
                 any_required = True
         if not any_required:

src/argilla/client/feedback/schemas.py

@@ -164,7 +164,9 @@ def update_settings(cls, values: Dict[str, Any]) -> Dict[str, Any]:
             ]
         if isinstance(values.get("labels"), list):
             values["settings"]["options"] = [{"value": label, "text": label} for label in values.get("labels")]
-        values["settings"]["visible_options"] = values.get("visible_labels", 20)
+        values["settings"]["visible_options"] = values.get(
+            "visible_labels"
+        )  # `None` is a possible value, which means all labels are visible
         return values
 
 

tests/client/feedback/test_dataset.py

@@ -18,7 +18,6 @@
 import datasets
 import pytest
 from argilla.client import api
-from pydantic import ValidationError
 
 if TYPE_CHECKING:
     from argilla.client.feedback.schemas import AllowedFieldTypes, AllowedQuestionTypes
@@ -84,6 +83,15 @@ def test_init_wrong_fields(feedback_dataset_guidelines: str, feedback_dataset_qu
             fields=[TextField(name="test", required=False)],
             questions=feedback_dataset_questions,
         )
+    with pytest.raises(ValueError, match="Expected `fields` to have unique names"):
+        FeedbackDataset(
+            guidelines=feedback_dataset_guidelines,
+            fields=[
+                TextField(name="test", required=True),
+                TextField(name="test", required=True),
+            ],
+            questions=feedback_dataset_questions,
+        )
 
 
 @pytest.mark.usefixtures("feedback_dataset_guidelines", "feedback_dataset_fields")
@@ -108,8 +116,17 @@ def test_init_wrong_questions(feedback_dataset_guidelines: str, feedback_dataset
             guidelines=feedback_dataset_guidelines,
             fields=feedback_dataset_fields,
             questions=[
-                TextQuestion(name="test", required=False),
-                RatingQuestion(name="test", values=[0, 1], required=False),
+                TextQuestion(name="question-1", required=False),
+                RatingQuestion(name="question-2", values=[0, 1], required=False),
+            ],
+        )
+    with pytest.raises(ValueError, match="Expected `questions` to have unique names"):
+        FeedbackDataset(
+            guidelines=feedback_dataset_guidelines,
+            fields=feedback_dataset_fields,
+            questions=[
+                TextQuestion(name="question-1", required=True),
+                TextQuestion(name="question-1", required=True),
             ],
         )
 

tests/client/feedback/test_schemas.py

@@ -88,6 +88,14 @@ def test_label_question_errors(
                 "visible_options": 5,
             },
         ),
+        (
+            {"name": "a", "description": "a", "required": True, "labels": ["a", "b"], "visible_labels": None},
+            {
+                "type": "label_selection",
+                "options": [{"value": "a", "text": "a"}, {"value": "b", "text": "b"}],
+                "visible_options": None,
+            },
+        ),
     ],
 )
 def test_label_question(schema_kwargs: Dict[str, Any], expected_settings: Dict[str, Any]) -> None:
@@ -163,6 +171,14 @@ def test_multi_label_question_errors(
                 "visible_options": 5,
             },
         ),
+        (
+            {"name": "a", "description": "a", "required": True, "labels": ["a", "b"], "visible_labels": None},
+            {
+                "type": "multi_label_selection",
+                "options": [{"value": "a", "text": "a"}, {"value": "b", "text": "b"}],
+                "visible_options": None,
+            },
+        ),
     ],
 )
 def test_multi_label_question(schema_kwargs: Dict[str, Any], expected_settings: Dict[str, Any]) -> None: