Skip to content

Commit

Permalink
[ENHANCEMENT] argilla: Remove attribute-like access (#5048)
Browse files Browse the repository at this point in the history
<!-- Thanks for your contribution! As part of our Community Growers
initiative 🌱, we're donating Justdiggit bunds in your name to reforest
sub-Saharan Africa. To claim your Community Growers certificate, please
contact David Berenstein in our Slack community or fill in this form
https://tally.so/r/n9XrxK once your PR has been merged. -->

# Description

This PR addresses
[discussion](#5026 (comment))
from PR #5026 and removes
attribute-like access for record fields, vectors and metadata.



**Type of change**

(Please delete options that are not relevant. Remember to title the PR
according to the type of change)

- [ ] New feature (non-breaking change which adds functionality)
- [ ] Refactor (change restructuring the codebase without changing
functionality)
- [ ] Improvement (change adding some improvement to an existing
functionality)

**How Has This Been Tested**

(Please describe the tests that you ran to verify your changes. And
ideally, reference `tests`)

- [ ] Test A
- [ ] Test B

**Checklist**

- [ ] 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
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] I filled out [the contributor form](https://tally.so/r/n9XrxK)
(see text above)
- [ ] I have added relevant notes to the `CHANGELOG.md` file (See
https://keepachangelog.com/)
  • Loading branch information
frascuchon authored Jun 18, 2024
1 parent 6452993 commit fa52a6e
Show file tree
Hide file tree
Showing 11 changed files with 45 additions and 93 deletions.
20 changes: 0 additions & 20 deletions argilla/src/argilla/records/_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -270,12 +270,6 @@ class RecordFields(dict):
def __init__(self, fields: Optional[Dict[str, FieldValue]] = None) -> None:
super().__init__(fields or {})

def __getattr__(self, item: str):
return self[item]

def __setattr__(self, key: str, value: MetadataValue):
self[key] = value

def to_dict(self) -> dict:
return dict(self.items())

Expand All @@ -286,12 +280,6 @@ class RecordMetadata(dict):
def __init__(self, metadata: Optional[Dict[str, MetadataValue]] = None) -> None:
super().__init__(metadata or {})

def __getattr__(self, item: str):
return self[item]

def __setattr__(self, key: str, value: MetadataValue):
self[key] = value

def to_dict(self) -> dict:
return dict(self.items())

Expand All @@ -307,12 +295,6 @@ class RecordVectors(dict):
def __init__(self, vectors: Dict[str, VectorValue]) -> None:
super().__init__(vectors or {})

def __getattr__(self, item: str):
return self[item]

def __setattr__(self, key: str, value: VectorValue):
self[key] = value

def to_dict(self) -> Dict[str, List[float]]:
return dict(self.items())

Expand Down Expand Up @@ -371,7 +353,6 @@ def api_models(self) -> List[UserResponseModel]:
]



class RecordSuggestions(Iterable[Suggestion]):
"""This is a container class for the suggestions of a Record.
It allows for accessing suggestions by attribute and iterating over them.
Expand Down Expand Up @@ -410,4 +391,3 @@ def to_dict(self) -> Dict[str, List[str]]:

def api_models(self) -> List[SuggestionModel]:
return [suggestion.api_model() for suggestion in self.__suggestions]

20 changes: 10 additions & 10 deletions argilla/tests/integration/test_add_records.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ def test_add_records(client):
assert dataset_records[0].id == str(mock_data[0]["id"])
assert dataset_records[1].id == str(mock_data[1]["id"])
assert dataset_records[2].id == str(mock_data[2]["id"])
assert dataset_records[0].fields.text == mock_data[0]["text"]
assert dataset_records[1].fields.text == mock_data[1]["text"]
assert dataset_records[2].fields.text == mock_data[2]["text"]
assert dataset_records[0].fields["text"] == mock_data[0]["text"]
assert dataset_records[1].fields["text"] == mock_data[1]["text"]
assert dataset_records[2].fields["text"] == mock_data[2]["text"]


def test_add_dict_records(client: Argilla):
Expand Down Expand Up @@ -124,7 +124,7 @@ def test_add_dict_records(client: Argilla):

for record, data in zip(ds.records, mock_data):
assert record.id == data["id"]
assert record.fields.text == data["text"]
assert record.fields["text"] == data["text"]
assert "label" not in record.__dict__

for record, data in zip(ds.records(batch_size=1, with_suggestions=True), mock_data):
Expand Down Expand Up @@ -193,7 +193,7 @@ def test_add_records_with_suggestions(client) -> None:
assert dataset_records[0].suggestions.topics.value == ["topic1", "topic2"]
assert dataset_records[0].suggestions.topics.score == [0.9, 0.8]

assert dataset_records[1].fields.text == mock_data[1]["text"]
assert dataset_records[1].fields["text"] == mock_data[1]["text"]
assert dataset_records[1].suggestions.comment.value == "I'm doing great, thank you!"
assert dataset_records[1].suggestions.comment.score is None
assert dataset_records[1].suggestions.topics.value == ["topic3"]
Expand Down Expand Up @@ -258,7 +258,7 @@ def test_add_records_with_responses(client) -> None:

for record, mock_record in zip(dataset_records, mock_data):
assert record.id == str(mock_record["id"])
assert record.fields.text == mock_record["text"]
assert record.fields["text"] == mock_record["text"]
assert record.responses.label[0].value == mock_record["my_label"]
assert record.responses.label[0].user_id == user.id

Expand Down Expand Up @@ -319,7 +319,7 @@ def test_add_records_with_responses_and_suggestions(client) -> None:
dataset_records = list(dataset.records(with_suggestions=True))

assert dataset_records[0].id == str(mock_data[0]["id"])
assert dataset_records[1].fields.text == mock_data[1]["text"]
assert dataset_records[1].fields["text"] == mock_data[1]["text"]
assert dataset_records[2].suggestions.label.value == "positive"
assert dataset_records[2].responses.label[0].value == "negative"
assert dataset_records[2].responses.label[0].user_id == user.id
Expand Down Expand Up @@ -386,7 +386,7 @@ def test_add_records_with_fields_mapped(client) -> None:
dataset_records = list(dataset.records(with_suggestions=True))

assert dataset_records[0].id == str(mock_data[0]["id"])
assert dataset_records[1].fields.text == mock_data[1]["x"]
assert dataset_records[1].fields["text"] == mock_data[1]["x"]
assert dataset_records[2].suggestions.label.value == "positive"
assert dataset_records[2].suggestions.label.score == 0.5
assert dataset_records[2].responses.label[0].value == "negative"
Expand Down Expand Up @@ -447,7 +447,7 @@ def test_add_records_with_id_mapped(client) -> None:
dataset_records = list(dataset.records(with_suggestions=True))

assert dataset_records[0].id == str(mock_data[0]["uuid"])
assert dataset_records[1].fields.text == mock_data[1]["x"]
assert dataset_records[1].fields["text"] == mock_data[1]["x"]
assert dataset_records[2].suggestions.label.value == "positive"
assert dataset_records[2].responses.label[0].value == "negative"
assert dataset_records[2].responses.label[0].user_id == user.id
Expand Down Expand Up @@ -571,7 +571,7 @@ def test_add_records_with_responses_and_same_schema_name(client: Argilla):

dataset_records = list(dataset.records(with_responses=True))

assert dataset_records[0].fields.text == mock_data[1]["text"]
assert dataset_records[0].fields["text"] == mock_data[1]["text"]
assert dataset_records[1].responses.label[0].value == "negative"
assert dataset_records[1].responses.label[0].user_id == user.id

Expand Down
2 changes: 1 addition & 1 deletion argilla/tests/integration/test_export_dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ def test_import_dataset_from_disk(dataset: rg.Dataset, client):
new_dataset = rg.Dataset.from_disk(output_dir, client=client)

for i, record in enumerate(new_dataset.records(with_suggestions=True)):
assert record.fields.text == mock_data[i]["text"]
assert record.fields["text"] == mock_data[i]["text"]
assert record.suggestions.label.value == mock_data[i]["label"]

assert new_dataset.settings.fields[0].name == "text"
Expand Down
4 changes: 2 additions & 2 deletions argilla/tests/integration/test_export_records.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ def test_export_records_from_json(dataset: rg.Dataset):
dataset.records.from_json(path=temp_file)

for i, record in enumerate(dataset.records(with_suggestions=True)):
assert record.fields.text == mock_data[i]["text"]
assert record.fields["text"] == mock_data[i]["text"]
assert record.suggestions.label.value == mock_data[i]["label"]
assert record.id == str(mock_data[i]["id"])

Expand Down Expand Up @@ -297,6 +297,6 @@ def test_import_records_from_hf_dataset(dataset: rg.Dataset) -> None:
dataset.records.log(records=mock_hf_dataset)

for i, record in enumerate(dataset.records(with_suggestions=True)):
assert record.fields.text == mock_data[i]["text"]
assert record.fields["text"] == mock_data[i]["text"]
assert record.suggestions.label.value == mock_data[i]["label"]
assert record.id == str(mock_data[i]["id"])
2 changes: 0 additions & 2 deletions argilla/tests/integration/test_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ def test_add_record_with_metadata(dataset_with_metadata: Dataset):
dataset_with_metadata.records.log(records)

for idx, record in enumerate(dataset_with_metadata.records):
assert record.metadata.category == records[idx]["category"]
assert record.metadata["category"] == records[idx]["category"]
assert len(record.metadata) == 1
models = record.metadata.api_models()
Expand All @@ -134,7 +133,6 @@ def test_add_record_with_mapped_metadata(dataset_with_metadata: Dataset):
dataset_with_metadata.records.log(records, mapping={"my_category": "category"})

for idx, record in enumerate(dataset_with_metadata.records):
assert record.metadata.category == records[idx]["my_category"]
assert record.metadata["category"] == records[idx]["my_category"]
assert len(record.metadata) == 1
models = record.metadata.api_models()
Expand Down
4 changes: 2 additions & 2 deletions argilla/tests/integration/test_query_records.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,12 @@ def test_query_records_by_text(client: Argilla, dataset: Dataset):

assert len(records) == 1
assert records[0].id == "1"
assert records[0].fields.text == "First record"
assert records[0].fields["text"] == "First record"

records = list(dataset.records(query="second"))
assert len(records) == 1
assert records[0].id == "2"
assert records[0].fields.text == "Second record"
assert records[0].fields["text"] == "Second record"

records = list(dataset.records(query="record"))
assert len(records) == 2
Expand Down
8 changes: 4 additions & 4 deletions argilla/tests/integration/test_update_dataset_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ def test_update_settings(self, client: Argilla, dataset: Dataset):

dataset = client.datasets(dataset.name)
settings = dataset.settings
assert settings.fields.text.use_markdown is True
assert settings.vectors.vector.dimensions == 10
assert settings.fields["text"].use_markdown is True
assert settings.vectors["vector"].dimensions == 10
assert isinstance(settings.metadata.metadata, FloatMetadataProperty)

settings.vectors.vector.title = "A new title for vector"
settings.vectors["vector"].title = "A new title for vector"

settings.update()
dataset = client.datasets(dataset.name)
assert dataset.settings.vectors.vector.title == "A new title for vector"
assert dataset.settings.vectors["vector"].title == "A new title for vector"
18 changes: 9 additions & 9 deletions argilla/tests/integration/test_vectors.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,9 @@ def test_vectors(client: rg.Argilla, dataset: rg.Dataset):
assert dataset_records[0].id == str(mock_data[0]["id"])
assert dataset_records[1].id == str(mock_data[1]["id"])
assert dataset_records[2].id == str(mock_data[2]["id"])
assert dataset_records[0].vectors.vector == mock_data[0]["vector"]
assert dataset_records[1].vectors.vector == mock_data[1]["vector"]
assert dataset_records[2].vectors.vector == mock_data[2]["vector"]
assert dataset_records[0].vectors["vector"] == mock_data[0]["vector"]
assert dataset_records[1].vectors["vector"] == mock_data[1]["vector"]
assert dataset_records[2].vectors["vector"] == mock_data[2]["vector"]


def test_vectors_return_with_bool(client: rg.Argilla, dataset: rg.Dataset):
Expand Down Expand Up @@ -106,9 +106,9 @@ def test_vectors_return_with_bool(client: rg.Argilla, dataset: rg.Dataset):
assert dataset_records[0].id == str(mock_data[0]["id"])
assert dataset_records[1].id == str(mock_data[1]["id"])
assert dataset_records[2].id == str(mock_data[2]["id"])
assert dataset_records[0].vectors.vector == mock_data[0]["vector"]
assert dataset_records[1].vectors.vector == mock_data[1]["vector"]
assert dataset_records[2].vectors.vector == mock_data[2]["vector"]
assert dataset_records[0].vectors["vector"] == mock_data[0]["vector"]
assert dataset_records[1].vectors["vector"] == mock_data[1]["vector"]
assert dataset_records[2].vectors["vector"] == mock_data[2]["vector"]


def test_vectors_return_with_name(client: rg.Argilla, dataset: rg.Dataset):
Expand Down Expand Up @@ -138,6 +138,6 @@ def test_vectors_return_with_name(client: rg.Argilla, dataset: rg.Dataset):
assert dataset_records[0].id == str(mock_data[0]["id"])
assert dataset_records[1].id == str(mock_data[1]["id"])
assert dataset_records[2].id == str(mock_data[2]["id"])
assert dataset_records[0].vectors.vector == mock_data[0]["vector"]
assert dataset_records[1].vectors.vector == mock_data[1]["vector"]
assert dataset_records[2].vectors.vector == mock_data[2]["vector"]
assert dataset_records[0].vectors["vector"] == mock_data[0]["vector"]
assert dataset_records[1].vectors["vector"] == mock_data[1]["vector"]
assert dataset_records[2].vectors["vector"] == mock_data[2]["vector"]
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def test_export_record_to_from_dict(record):
assert record.suggestions[0].value == imported_record.suggestions[0].value
for key, value in record.metadata.items():
assert imported_record.metadata[key] == value
assert record.fields.text == imported_record.fields.text
assert record.fields["text"] == imported_record.fields["text"]
# This is a consequence of how UUIDs are treated in python and could be
# problematic for users.
assert str(record.id) == imported_record.id
Expand All @@ -62,5 +62,5 @@ def test_export_generic_io_via_json(record):
assert record.suggestions[0].value == imported_record.suggestions[0].value
for key, value in record.metadata.items():
assert imported_record.metadata[key] == value
assert record.fields.text == imported_record.fields.text
assert record.vectors.text == imported_record.vectors.text
assert record.fields["text"] == imported_record.fields["text"]
assert record.vectors["text"] == imported_record.vectors["text"]
28 changes: 14 additions & 14 deletions argilla/tests/unit/test_record_ingestion.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def test_ingest_record_from_dict(dataset):
},
)

assert record.fields.prompt == "What is the capital of France?"
assert record.fields["prompt"] == "What is the capital of France?"
assert record.suggestions.label.value == "positive"


Expand All @@ -58,7 +58,7 @@ def test_ingest_record_from_dict_with_mapping(dataset):
},
)

assert record.fields.prompt == "What is the capital of France?"
assert record.fields["prompt"] == "What is the capital of France?"
assert record.suggestions.label.value == "positive"


Expand All @@ -70,7 +70,7 @@ def test_ingest_record_from_dict_with_suggestions(dataset):
},
)

assert record.fields.prompt == "Hello World, how are you?"
assert record.fields["prompt"] == "Hello World, how are you?"
assert record.suggestions.label.value == "negative"


Expand All @@ -88,7 +88,7 @@ def test_ingest_record_from_dict_with_suggestions_scores(dataset):
},
)

assert record.fields.prompt == "Hello World, how are you?"
assert record.fields["prompt"] == "Hello World, how are you?"
assert record.suggestions.label.value == "negative"
assert record.suggestions.label.score == 0.9
assert record.suggestions.label.agent == "model_name"
Expand All @@ -108,7 +108,7 @@ def test_ingest_record_from_dict_with_suggestions_scores_and_agent(dataset):
},
)

assert record.fields.prompt == "Hello World, how are you?"
assert record.fields["prompt"] == "Hello World, how are you?"
assert record.suggestions.label.value == "negative"
assert record.suggestions.label.score == 0.9
assert record.suggestions.label.agent == "model_name"
Expand All @@ -127,7 +127,7 @@ def test_ingest_record_from_dict_with_responses(dataset):
user_id=user_id,
)

assert record.fields.prompt == "Hello World, how are you?"
assert record.fields["prompt"] == "Hello World, how are you?"
assert record.responses.label[0].value == "negative"
assert record.responses.label[0].user_id == user_id

Expand All @@ -142,7 +142,7 @@ def test_ingest_record_from_dict_with_id_as_id(dataset):
},
)

assert record.fields.prompt == "Hello World, how are you?"
assert record.fields["prompt"] == "Hello World, how are you?"
assert record.id == record_id


Expand All @@ -159,7 +159,7 @@ def test_ingest_record_from_dict_with_id_and_mapping(dataset):
},
)

assert record.fields.prompt == "Hello World, how are you?"
assert record.fields["prompt"] == "Hello World, how are you?"
assert record.id == record_id


Expand All @@ -172,7 +172,7 @@ def test_ingest_record_from_dict_with_metadata(dataset):
},
)

assert record.fields.prompt == "Hello World, how are you?"
assert record.fields["prompt"] == "Hello World, how are you?"
assert record.suggestions.label.value == "negative"
assert record.metadata["score"] == 0.9

Expand All @@ -189,7 +189,7 @@ def test_ingest_record_from_dict_with_metadata_and_mapping(dataset):
},
)

assert record.fields.prompt == "Hello World, how are you?"
assert record.fields["prompt"] == "Hello World, how are you?"
assert record.suggestions.label.value == "negative"
assert record.metadata["score"] == 0.9

Expand All @@ -203,9 +203,9 @@ def test_ingest_record_from_dict_with_vectors(dataset):
},
)

assert record.fields.prompt == "Hello World, how are you?"
assert record.fields["prompt"] == "Hello World, how are you?"
assert record.suggestions.label.value == "negative"
assert record.vectors.vector == [1, 2, 3]
assert record.vectors["vector"] == [1, 2, 3]


def test_ingest_record_from_dict_with_vectors_and_mapping(dataset):
Expand All @@ -220,6 +220,6 @@ def test_ingest_record_from_dict_with_vectors_and_mapping(dataset):
},
)

assert record.fields.prompt == "Hello World, how are you?"
assert record.fields["prompt"] == "Hello World, how are you?"
assert record.suggestions.label.value == "negative"
assert record.vectors.vector == [1, 2, 3]
assert record.vectors["vector"] == [1, 2, 3]
Loading

0 comments on commit fa52a6e

Please sign in to comment.