Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DRAFT] [REFACTOR] Iterate settings management from the SDK #5564

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions argilla/src/argilla/_api/_questions.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,12 @@ def update(
question: QuestionModel,
) -> QuestionModel:
# TODO: Implement update method for fields with server side ID
raise NotImplementedError
return question

@api_error_handler
def delete(self, question_id: UUID) -> None:
# TODO: Implement delete method for fields with server side ID
raise NotImplementedError
pass

####################
# Utility methods #
Expand Down
32 changes: 23 additions & 9 deletions argilla/src/argilla/datasets/_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,15 +151,24 @@ def get(self) -> "Dataset":
self.settings.get()
return self

def create(self) -> "Dataset":
def create(self, publish: bool = True) -> "Dataset":
"""Creates the dataset on the server with the `Settings` configuration.

Parameters:
publish (bool): If True, the dataset is published after creation. Default is True.

Returns:
Dataset: The created dataset object.

"""
super().create()
try:
return self._publish()
super().create()
self._settings.create()

if publish:
return self.publish()

return self.get()
except Exception as e:
self._log_message(message=f"Error creating dataset: {e}", level="error")
self._rollback_dataset_creation()
Expand All @@ -174,6 +183,17 @@ def update(self) -> "Dataset":
self.settings.update()
return self

def publish(self) -> "Dataset":
"""
Publishes the dataset on the server.

Returns:
The published dataset object.

"""
self._api.publish(dataset_id=self._model.id)
return self.get()

def progress(self, with_users_distribution: bool = False) -> dict:
"""Returns the team's progress on the dataset.

Expand Down Expand Up @@ -236,12 +256,6 @@ def api_model(self) -> DatasetModel:
self._model.workspace_id = self.workspace.id
return self._model

def _publish(self) -> "Dataset":
self._settings.create()
# self._api.publish(dataset_id=self._model.id)

return self.get()

def _resolve_workspace(self) -> Workspace:
workspace = self._workspace

Expand Down
59 changes: 51 additions & 8 deletions argilla/src/argilla/settings/_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import json
import os
import re
from functools import cached_property
from pathlib import Path
from typing import List, Optional, TYPE_CHECKING, Dict, Union, Iterator, Sequence, Literal
from uuid import UUID
Expand Down Expand Up @@ -160,7 +159,8 @@ def dataset(self, dataset: "Dataset"):
self._dataset = dataset
self._client = dataset._client

@cached_property
# TODO: The schema should be refreshed after a change on the settings names
@property
def schema(self) -> dict:
schema_dict = {}

Expand All @@ -178,7 +178,7 @@ def schema(self) -> dict:

return schema_dict

@cached_property
@property
def schema_by_id(self) -> Dict[UUID, Union[Field, QuestionType, MetadataType, VectorField]]:
return {v.id: v for v in self.schema.values()}

Expand Down Expand Up @@ -219,7 +219,7 @@ def update(self) -> "Resource":
self.__fields.update()
self.__vectors.update()
self.__metadata.update()
# self.questions.update()
self.questions.update()

self._update_last_api_call()
return self
Expand Down Expand Up @@ -385,9 +385,8 @@ def _update_dataset_related_attributes(self):
self._client.api.datasets.update(dataset_model)

def _validate_empty_settings(self):
if not all([self.fields, self.questions]):
message = "Fields and questions are required"
raise SettingsError(message=message)
if not all(self.fields):
raise SettingsError(message="At least one field must be defined in the settings")

def _validate_duplicate_names(self) -> None:
dataset_properties_by_name = {}
Expand Down Expand Up @@ -488,6 +487,16 @@ def add(self, property: Property) -> Property:
setattr(self, property.name, property)
return property

def remove(self, property: Union[str, Property]) -> None:
if isinstance(property, str):
property = self._properties_by_name.get(property)

if property is None:
return

property = self._properties_by_name.pop(property.name)
property.delete()

def create(self):
for property in self:
try:
Expand Down Expand Up @@ -530,16 +539,50 @@ class to work with questions as we do with fields, vectors, or metadata (special
Once issue https://github.com/argilla-io/argilla/issues/4931 is tackled, this class should be removed.
"""

def remove(self, property: Union[str, Property]) -> None:
if isinstance(property, str):
property = self._properties_by_name.get(property)

if property is None:
return

property = self._properties_by_name.pop(property.name)
self._delete_question(property)

# TODO: Align to the Resource model
def create(self):
for question in self:
try:
self._create_question(question)
except ArgillaAPIError as e:
raise SettingsError(f"Failed to create question {question.name}") from e

def _create_question(self, question: QuestionType) -> None:
# TODO: Align to the Resource model
def update(self):
for item in self:
try:
item.dataset = self._settings.dataset
self._update_question(item) if item.id else self._create_question(item)
except ArgillaAPIError as e:
raise SettingsError(f"Failed to update {item.name!r}: {e.message}") from e

# TODO: Align to the Resource model
def _create_question(self, question: QuestionType) -> QuestionType:
question_model = self._settings._client.api.questions.create(
dataset_id=self._settings.dataset.id,
question=question.api_model(),
)
question._model = question_model

return question

# TODO: Align to the Resource model
def _update_question(self, question: QuestionType) -> QuestionType:
question_model = self._settings._client.api.questions.update(question.api_model())
question._model = question_model

return question

# TODO: Align to the Resource model
def _delete_question(self, question: QuestionType) -> None:
self._settings._client.api.questions.delete(question.id)
Loading