Skip to content

Commit

Permalink
[BUGFIX] argilla: prevent enum literal validation errors (#5679)
Browse files Browse the repository at this point in the history
# Description
<!-- Please include a summary of the changes and the related issue.
Please also include relevant motivation and context. List any
dependencies that are required for this change. -->

In some environments, working with enun instances as string values may
lead to some validation errors. See [this discord
discussion](https://discord.com/channels/879548962464493619/1300936213531983882/1300936213531983882).

This PR reviews how metadata settings classes are defined, and defines
type directly as a literal string, in the same way we do fo questions or
fields, instead of using the MetadataPropertyType class.

**Type of change**
<!-- Please delete options that are not relevant. Remember to title the
PR according to the type of change -->

- Bug fix (non-breaking change which fixes an issue)
- Refactor (change restructuring the codebase without changing
functionality)

**How Has This Been Tested**
<!-- Please add some reference about how your feature has been tested.
-->

**Checklist**
<!-- Please go over the list and make sure you've taken everything into
account -->

- I added relevant documentation
- I followed the style guidelines of this project
- I did a self-review of my code
- I made corresponding changes to the documentation
- I confirm My changes generate no new warnings
- I have added tests that prove my fix is effective or that my feature
works
- I have added relevant notes to the CHANGELOG.md file (See
https://keepachangelog.com/)
  • Loading branch information
frascuchon authored Dec 11, 2024
1 parent 0cbbdeb commit 1a34f54
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 50 deletions.
1 change: 0 additions & 1 deletion argilla/src/argilla/_models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@
)
from argilla._models._settings._metadata import (
MetadataFieldModel,
MetadataPropertyType,
BaseMetadataPropertySettings,
TermsMetadataPropertySettings,
NumericMetadataPropertySettings,
Expand Down
26 changes: 7 additions & 19 deletions argilla/src/argilla/_models/_settings/_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from enum import Enum
from typing import List, Literal, Optional, Union, Annotated, Any
from uuid import UUID

Expand All @@ -22,19 +21,12 @@
from argilla._models import ResourceModel


class MetadataPropertyType(str, Enum):
terms = "terms"
integer = "integer"
float = "float"


class BaseMetadataPropertySettings(BaseModel):
type: MetadataPropertyType
visible_for_annotators: Optional[bool] = True


class TermsMetadataPropertySettings(BaseMetadataPropertySettings):
type: Literal[MetadataPropertyType.terms]
type: Literal["terms"] = "terms"
values: Optional[List[Any]] = None

@field_validator("values")
Expand Down Expand Up @@ -64,7 +56,7 @@ def __validate_min_max(cls, values):


class IntegerMetadataPropertySettings(NumericMetadataPropertySettings):
type: Literal[MetadataPropertyType.integer]
type: Literal["integer"] = "integer"

@model_validator(mode="before")
@classmethod
Expand All @@ -78,7 +70,7 @@ def __validate_min_max(cls, values):


class FloatMetadataPropertySettings(NumericMetadataPropertySettings):
type: Literal[MetadataPropertyType.float]
type: Literal["float"] = "float"


MetadataPropertySettings = Annotated[
Expand All @@ -97,12 +89,15 @@ class MetadataFieldModel(ResourceModel):
name: str
settings: MetadataPropertySettings

type: Optional[MetadataPropertyType] = Field(None, validate_default=True)
title: Optional[str] = None
visible_for_annotators: Optional[bool] = True

dataset_id: Optional[UUID] = None

@property
def type(self) -> str:
return self.settings.type

@field_validator("title")
@classmethod
def __title_default(cls, title, values):
Expand All @@ -112,10 +107,3 @@ def __title_default(cls, title, values):
@field_serializer("id", "dataset_id", when_used="unless-none")
def serialize_id(self, value: UUID) -> str:
return str(value)

@field_validator("type", mode="plain")
@classmethod
def __validate_type(cls, type, values):
if type is None:
return values.data["settings"].type
return type
39 changes: 11 additions & 28 deletions argilla/src/argilla/settings/_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
from argilla._api._metadata import MetadataAPI
from argilla._exceptions import MetadataError
from argilla._models import (
MetadataPropertyType,
TermsMetadataPropertySettings,
FloatMetadataPropertySettings,
IntegerMetadataPropertySettings,
Expand Down Expand Up @@ -122,13 +121,12 @@ def __init__(
super().__init__(client=client)

try:
settings = TermsMetadataPropertySettings(values=options, type=MetadataPropertyType.terms)
settings = TermsMetadataPropertySettings(values=options)
except ValueError as e:
raise MetadataError(f"Error defining metadata settings for {name}") from e

self._model = MetadataFieldModel(
name=name,
type=MetadataPropertyType.terms,
title=title,
settings=settings,
visible_for_annotators=visible_for_annotators,
Expand Down Expand Up @@ -176,13 +174,12 @@ def __init__(
super().__init__(client=client)

try:
settings = FloatMetadataPropertySettings(min=min, max=max, type=MetadataPropertyType.float)
settings = FloatMetadataPropertySettings(min=min, max=max)
except ValueError as e:
raise MetadataError(f"Error defining metadata settings for {name}") from e

self._model = MetadataFieldModel(
name=name,
type=MetadataPropertyType.float,
title=title,
settings=settings,
visible_for_annotators=visible_for_annotators,
Expand Down Expand Up @@ -237,13 +234,12 @@ def __init__(
super().__init__(client=client)

try:
settings = IntegerMetadataPropertySettings(min=min, max=max, type=MetadataPropertyType.integer)
settings = IntegerMetadataPropertySettings(min=min, max=max)
except ValueError as e:
raise MetadataError(f"Error defining metadata settings for {name}") from e

self._model = MetadataFieldModel(
name=name,
type=MetadataPropertyType.integer,
title=title,
settings=settings,
visible_for_annotators=visible_for_annotators,
Expand Down Expand Up @@ -283,27 +279,14 @@ def from_model(cls, model: MetadataFieldModel) -> "IntegerMetadataProperty":
class MetadataField:
@classmethod
def from_model(cls, model: MetadataFieldModel) -> MetadataType:
switch = {
MetadataPropertyType.terms: TermsMetadataProperty,
MetadataPropertyType.float: FloatMetadataProperty,
MetadataPropertyType.integer: IntegerMetadataProperty,
}
metadata_type = model.type
try:
return switch[metadata_type].from_model(model)
except KeyError as e:
raise MetadataError(f"Unknown metadata property type: {metadata_type}") from e
if model.type == "terms":
return TermsMetadataProperty.from_model(model)
elif model.type == "float":
return FloatMetadataProperty.from_model(model)
elif model.type == "integer":
return IntegerMetadataProperty.from_model(model)
raise MetadataError(f"Unknown metadata property type: {model.type}")

@classmethod
def from_dict(cls, data: dict) -> MetadataType:
switch = {
MetadataPropertyType.terms: TermsMetadataProperty,
MetadataPropertyType.float: FloatMetadataProperty,
MetadataPropertyType.integer: IntegerMetadataProperty,
}
metadata_type = data["type"]
try:
metadata_model = MetadataFieldModel(**data)
return switch[metadata_type].from_model(metadata_model)
except KeyError as e:
raise MetadataError(f"Unknown metadata property type: {metadata_type}") from e
return cls.from_model(MetadataFieldModel(**data))
2 changes: 0 additions & 2 deletions argilla/tests/unit/test_settings/test_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ def test_create_metadata_terms(self, options: list):
"name": "metadata",
"settings": {"type": "terms", "values": options, "visible_for_annotators": True},
"title": "A metadata property",
"type": "terms",
"visible_for_annotators": True,
"inserted_at": None,
"updated_at": None,
Expand All @@ -59,7 +58,6 @@ def test_create_terms_metadata_without_options(self):
"name": "metadata",
"title": "metadata",
"settings": {"type": "terms", "values": None, "visible_for_annotators": True},
"type": "terms",
"visible_for_annotators": True,
"inserted_at": None,
"updated_at": None,
Expand Down

0 comments on commit 1a34f54

Please sign in to comment.