Skip to content

Commit

Permalink
[BUGFIX] Fetch dataset setting when iterate client.datasets (#5753)
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. -->

Closes #5718

**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)

**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 13, 2024
1 parent a74d3eb commit ebb0fa2
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 3 deletions.
4 changes: 4 additions & 0 deletions argilla/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ These are the section headers that we use:

## [Unreleased]()

### Fixed

- Fixed error when iterating over datasets and settings are not properly loaded. ([#5753](https://github.com/argilla-io/argilla/pull/5753))

## [2.5.0](https://github.com/argilla-io/argilla/compare/v2.4.0...v2.5.0)

### Added
Expand Down
6 changes: 4 additions & 2 deletions argilla/src/argilla/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,9 @@ class Datasets(Sequence["Dataset"], ResourceHTMLReprMixin):
"""A collection of datasets. It can be used to create a new dataset or to get an existing one."""

class _Iterator(GenericIterator["Dataset"]):
pass
def __next__(self):
dataset = super().__next__()
return dataset.get()

def __init__(self, client: "Argilla") -> None:
self._client = client
Expand Down Expand Up @@ -366,7 +368,7 @@ def __getitem__(self, index: slice) -> Sequence["Dataset"]: ...

def __getitem__(self, index) -> "Dataset":
model = self._api.list()[index]
return self._from_model(model)
return self._from_model(model).get()

def __len__(self) -> int:
return len(self._api.list())
Expand Down
27 changes: 26 additions & 1 deletion argilla/tests/integration/test_listing_datasets.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from argilla import Argilla, Dataset, Settings, TextField, TextQuestion, Workspace
from argilla import Argilla, Dataset, Settings, TextField, TextQuestion, Workspace, TaskDistribution


class TestDatasetsList:
Expand All @@ -33,3 +33,28 @@ def test_list_datasets(self, client: Argilla):
for ds in datasets:
if ds.name == "test_dataset":
assert ds == dataset, "The dataset was not loaded properly"

def test_list_dataset_with_custom_task_distribution(self, client: Argilla, workspace: Workspace):
dataset = Dataset(
name="test_dataset",
workspace=workspace.name,
settings=Settings(
fields=[TextField(name="text")],
questions=[TextQuestion(name="text_question")],
distribution=TaskDistribution(min_submitted=4),
),
client=client,
)
dataset.create()
datasets = client.datasets
assert len(datasets) > 0, "No datasets were found"

dataset_idx = 0
for idx, ds in enumerate(datasets):
if ds.id == dataset.id:
dataset_idx = idx
assert ds.settings.distribution.min_submitted == 4, "The dataset was not loaded properly"
break

ds = client.datasets[dataset_idx]
assert ds.settings.distribution.min_submitted == 4, "The dataset was not loaded properly"

0 comments on commit ebb0fa2

Please sign in to comment.