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

feat: Allow exporting data for SFT, Reward Modelling (related to RLHF), DPO, rename TrainingTaskMapping #3467

Merged
merged 89 commits into from
Aug 28, 2023

Conversation

tomaarsen
Copy link
Contributor

@tomaarsen tomaarsen commented Jul 27, 2023

Resolves #3379, resolves #3377
Closes #3522

Hello!

Pull Request overview

  • Prepare data for SFT, RM, DPO in TRL.
  • Rename TrainingTaskMapping to TrainingTask and task_mapping to task.

Description

Prepare data

from argilla.feedback import TrainingTask

def formatting_func(sample: Dict[str, Any]):
    ...
    yield template.format(
        prompt=sample["prompt"],
        response=sample["response"],
    )

task = TrainingTask.for_supervised_fine_tuning(formatting_func=formatting_func)
ds = fds_dataset.prepare_for_training(framework="trl", task=task)
# -> ds has "text" and "id" columns

Compatible with SFTTrainer.

task = TrainingTask.for_reward_modelling(chosen_rejected_func=chosen_rejected_func)
ds = fds_dataset.prepare_for_training(framework="trl", task=task)
# -> ds has "chosen" and "rejected" columns

Nearly compatible with RewardTrainer.

task = TrainingTask.for_direct_preference_optimization(prompt_chosen_rejected_func=prompt_chosen_rejected_func)
ds = fds_dataset.prepare_for_training(framework="trl", task=task)
# -> ds has "prompt", "chosen" and "rejected" columns

Compatible with DPOTrainer.

Details

I implement this by calling dataset.format_as("datasets") and then passing each sample (a simple dictionary) from this dataset to the function that the user provides. This user provided function can return None, one sample, a list of samples, or yield samples. This allows users to export multiple training samples from a single Argilla record, e.g. when there's multiple annotators that provided useful corrections, or if the annotated record justifies 3 "chosen", "rejected" pairs because there's a ranking between 3 texts.

Rename

TrainingTaskMapping is now TrainingTask - the "mapping" part is just unintuitive to the user. Same for task_mapping to task. Note: If people used task_mapping=... before, that will now fail. I can make this deprecation softer, but then I have to make task optional, which I would rather not do.

TODO:

  • Add TRL to ArgillaTrainer, allowing:
    task = TrainingTask.for_supervised_fine_tuning(
        formatting_func=formatting_func
    ) # or any other task from this PR
    trainer = ArgillaTrainer(
        dataset=fds_dataset,
        task=task,
        framework="trl",
    )
    trainer.train()
  • Consider renaming FeedbackDataset.prepare_for_training to FeedbackDataset.export.
  • New tests
  • Add documentation

Type of change

  • New feature

How Has This Been Tested

Not finished yet.

Checklist

  • I added relevant documentation
  • follows 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 (see text above)
  • I have added relevant notes to the CHANGELOG.md file (See https://keepachangelog.com/)

  • Tom Aarsen

@codecov
Copy link

codecov bot commented Jul 27, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.47% 🎉

Comparison is base (6630d7b) 90.13% compared to head (d88e93b) 90.61%.
Report is 197 commits behind head on develop.

❗ Current head d88e93b differs from pull request most recent head 50b68f2. Consider uploading reports for the commit 50b68f2 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3467      +/-   ##
===========================================
+ Coverage    90.13%   90.61%   +0.47%     
===========================================
  Files          233      262      +29     
  Lines        12493    14137    +1644     
===========================================
+ Hits         11261    12810    +1549     
- Misses        1232     1327      +95     
Flag Coverage Δ
pytest ?

Flags with carried forward coverage won't be shown. Click here to find out more.

see 186 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tomaarsen
Copy link
Contributor Author

tomaarsen commented Aug 3, 2023

The CI is failing with this message. I've seen this before I think, is it expected?
image

Perhaps you've seen it before @gabrielmbmb?

@tomaarsen
Copy link
Contributor Author

tomaarsen commented Aug 25, 2023

TODO: Changelog

Edit: Done

@davidberenstein1957
Copy link
Member

@tomaarsen I restructured the docs. Could you add some final docs to docs/trl-docs-revisited?

davidberenstein1957 and others added 3 commits August 28, 2023 15:04
<!-- 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

I updated the docs for the `ArgillaTrainer` and the new tasks.

Closes #<issue_number>

**Type of change**

(Remember to title the PR according to the type of change)

- [X] Documentation update

**How Has This Been Tested**

(Please describe the tests that you ran to verify your changes.)

- [X] `sphinx-autobuild` (read [Developer
Documentation](https://docs.argilla.io/en/latest/community/developer_docs.html#building-the-documentation)
for more details)

**Checklist**

- [X] I added relevant documentation
- [X] I followed the style guidelines of this project
- [X] I did a self-review of my code
- [X] I made corresponding changes to the documentation
- [X] My changes generate no new warnings
- [ ] I filled out [the contributor form](https://tally.so/r/n9XrxK)
(see text above)
- [X] I have added relevant notes to the `CHANGELOG.md` file (See
https://keepachangelog.com/)

---------

Co-authored-by: Tom Aarsen <[email protected]>
Co-authored-by: Alvaro Bartolome <[email protected]>
…ing_func by changing tuple type check

tests: fix test_prepare_for_training_sft by proper pydantic typing
@davidberenstein1957 davidberenstein1957 merged commit 76d9a4b into argilla-io:develop Aug 28, 2023
4 of 15 checks passed
artikandri pushed a commit to CLARIN-PL/argilla that referenced this pull request Oct 30, 2023
…asetMixin` (argilla-io#3539)

# Description

This PR addresses the feature mentioned by @tomaarsen at
argilla-io#3467, to basically export the
`responses` for the existing questions in the `FeedbackDataset` when
calling `push_to_huggingface` in a row-based format instead of using the
`Sequence` from 🤗`datasets`. This makes the dataset from the HuggingFace
Hub more readable, and also easier to use with other frameworks and/or
libraries.

```diff
- {"user_id": ["A", "B"], "value": [1, 2], "status": ["C", "D"]}
+ [{"user_id": "A", "value": 1, "status": "C"}, {"user_id": "B", "value": 2, "status": "D"}]
```

Additionally, this PR also ensure that the backwards compatibility is
preserved with the previous versions, and assumes the new format as the
default one when calling `format_as("datasets")`.

Finally, this PR also solves an issue reported by @nataliaElv recently
that was affecting the `suggestions` when calling
`FeedbackDataset.from_argilla`, as those were just kept when there were
`responses`, otherwise, a `continue` statement was being called so the
`suggestions` were completely ignored.

**Type of change**

- [X] Bug fix (non-breaking change which fixes an issue)
- [X] New feature (non-breaking change which adds functionality)

**How Has This Been Tested**

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

- [X] Ran the following script and tested different combinations as the
connection to HuggingFace is mocked, but we should create a fake/testing
user at some point to avoid overloading `argilla-io`

```python
import argilla as rg

dataset = rg.FeedbackDataset(
    fields=[
        rg.TextField(
            name="prompt",
            required=True,
        ),
    ],
    questions=[
        rg.TextQuestion(
            name="response-edit",
            title="Add or edit the response if necessary",
            required=True,
        ),
    ],
)
dataset.add_records(
    rg.FeedbackRecord(
        fields={
            "prompt": "This is the prompt!",
        },
        suggestions=[
            {
                "question_name": "response-edit",
                "value": "This is the suggestion!"
            }
        ],
    )
)
dataset.push_to_huggingface("<REPO_ID>")
dataset = rg.FeedbackDataset.from_huggingface("<REPO_ID>")
assert dataset.records[0].suggestions is not None
```

**Checklist**

- [ ] I added relevant documentation
- [X] follows the style guidelines of this project
- [X] I did a self-review of my code
- [ ] I made corresponding changes to the documentation
- [X] 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)
- [X] I have added relevant notes to the CHANGELOG.md file (See
https://keepachangelog.com/)

---------

Co-authored-by: Gabriel Martin <[email protected]>
artikandri added a commit to CLARIN-PL/argilla that referenced this pull request Oct 30, 2023
* chore: bump version to `1.14.0`

* chore: add `argilla-office-hours` Calendly link back

* fix: `push_to_argilla` to return `RemoteFeedbackDataset` without re-assigning class (argilla-io#3508)

# Description

This PR adds a `warnings.warn` message to let the users know that the
returned object from `FeedbackDataset.push_to_argilla` needs to be
handled, otherwise the dataset instance will remain local, as
`push_to_argilla` with no arguments won't do anything.

So on, before we had to `push_to_argilla` a new `FeedbackDataset` with
name and/or workspaces, and then we could `push_to_argilla` with no
arguments to push the updates, but we no longer need to do that, as now
we can re-use the returned `FeedbackDataset` from `push_to_argilla` to
have a `FeedbackDataset` fully integrated with Argilla.

```diff
import argilla as rg

dataset = rg.FeedbackDataset(...)
- dataset.push_to_argilla(name="my-dataset", workspace="my-workspace")
+ remote_dataset = dataset.push_to_argilla(name="my-dataset", workspace="my-workspace")

dataset.add_records(...)
- dataset.push_to_argilla()
```

**Type of change**

- [X] Bug fix (non-breaking change which fixes an issue)
- [X] Breaking change (fix or feature that would cause existing
functionality to not work as expected)

**How Has This Been Tested**

- [X] Catch returned object in `push_to_argilla` and handle
`DeprecationWarning`

**Checklist**

- [ ] I added relevant documentation
- [X] follows the style guidelines of this project
- [X] I did a self-review of my code
- [ ] I made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [X] 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/)

---------

Co-authored-by: Gabriel Martín Blázquez <[email protected]>

* feat: add `delete` method to `FeedbackDataset` in Argilla (argilla-io#3512)

# Description

This PR adds a `delete` method for `RemoteFeedbackDataset` which is a
`FeedbackDataset` that has been pushed to Argilla. So on, the `delete`
method deletes a dataset in Argilla, but it's just available for `owner`
users and for `admin` users with that dataset within their workspace,
otherwise the method won't work and will raise a `PermissionError`.

So on, now both `owner` and `admin` users can delete a `FeedbackDataset`
from Argilla as:

```python
import argilla as rg

rg.init(...)

dataset = FeedbackDataset.from_argilla(...)
dataset.delete()
```

Or alternatively

```python
import argilla as rg

rg.init(...)

dataset = FeedbackDataset(...)
remote_dataset = dataset.push_to_argilla(...)
remote_dataset.delete()
```

Closes argilla-io#3413

**Type of change**

- [X] New feature (non-breaking change which adds functionality)

**How Has This Been Tested**

- [X] Add integration tests for `RemoteFeedbackDataset.delete` including
every role

**Checklist**

- [ ] I added relevant documentation
- [X] follows the style guidelines of this project
- [X] I did a self-review of my code
- [ ] I made corresponding changes to the documentation
- [X] My changes generate no new warnings
- [X] 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)
- [X] I have added relevant notes to the CHANGELOG.md file (See
https://keepachangelog.com/)

* fix: `publish_dataset` to check `required=True` on at least one field/question (argilla-io#3511)

# Description

This PR fixes a bug with the `PUT /api/v1/datasets/{dataset_id}/publish`
endpoint, as there was a missing check on the `required` flag for each
field/question which was allowing to publish `FeedbackTask` datasets
with no required fields and/or questions.

**Type of change**

- [X] Bug fix (non-breaking change which fixes an issue)

**How Has This Been Tested**

- [X] Update existing unit/integration tests
- [X] Add unit/integration tests with `required=True` and
`required=False`

**Checklist**

- [ ] I added relevant documentation
- [X] follows the style guidelines of this project
- [X] I did a self-review of my code
- [ ] I made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [X] 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/)

---------

Co-authored-by: Gabriel Martin <[email protected]>

* docs: fix link to ASGI middleware tutorial (argilla-io#3518)

# Description

Fix broken link to ASGI middleware tutorial.

**Type of change**

- [x] Documentation update

**How Has This Been Tested**

N/A

**Checklist**

- [ ] I added relevant documentation
- [x] follows the style guidelines of this project
- [x] I did a self-review of my code
- [ ] I made corresponding changes to the documentation
- [x] 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/)

* fix: `thread_ts` is not always present (argilla-io#3538)

# Description

The `slack-post-credentials` action is failing from time to time because
not all the messages have the `thread_ts` key. This PR fixes this issue
using the `ts` key instead of the `thread_ts`.

**Type of change**

- [x] Bug fix (non-breaking change which fixes an issue)

**How Has This Been Tested**

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

The deployment job got executed fine for this PR:
https://github.com/argilla-io/argilla/actions/runs/5818199084/job/15774977297?pr=3538

**Checklist**

- [x] I followed the style guidelines of this project
- [x] I did a self-review of my code
- [x] 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/)

* fix: restore `suggestions` and `responses` as rows in `HuggingFaceDatasetMixin` (argilla-io#3539)

# Description

This PR addresses the feature mentioned by @tomaarsen at
argilla-io#3467, to basically export the
`responses` for the existing questions in the `FeedbackDataset` when
calling `push_to_huggingface` in a row-based format instead of using the
`Sequence` from 🤗`datasets`. This makes the dataset from the HuggingFace
Hub more readable, and also easier to use with other frameworks and/or
libraries.

```diff
- {"user_id": ["A", "B"], "value": [1, 2], "status": ["C", "D"]}
+ [{"user_id": "A", "value": 1, "status": "C"}, {"user_id": "B", "value": 2, "status": "D"}]
```

Additionally, this PR also ensure that the backwards compatibility is
preserved with the previous versions, and assumes the new format as the
default one when calling `format_as("datasets")`.

Finally, this PR also solves an issue reported by @nataliaElv recently
that was affecting the `suggestions` when calling
`FeedbackDataset.from_argilla`, as those were just kept when there were
`responses`, otherwise, a `continue` statement was being called so the
`suggestions` were completely ignored.

**Type of change**

- [X] Bug fix (non-breaking change which fixes an issue)
- [X] New feature (non-breaking change which adds functionality)

**How Has This Been Tested**

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

- [X] Ran the following script and tested different combinations as the
connection to HuggingFace is mocked, but we should create a fake/testing
user at some point to avoid overloading `argilla-io`

```python
import argilla as rg

dataset = rg.FeedbackDataset(
    fields=[
        rg.TextField(
            name="prompt",
            required=True,
        ),
    ],
    questions=[
        rg.TextQuestion(
            name="response-edit",
            title="Add or edit the response if necessary",
            required=True,
        ),
    ],
)
dataset.add_records(
    rg.FeedbackRecord(
        fields={
            "prompt": "This is the prompt!",
        },
        suggestions=[
            {
                "question_name": "response-edit",
                "value": "This is the suggestion!"
            }
        ],
    )
)
dataset.push_to_huggingface("<REPO_ID>")
dataset = rg.FeedbackDataset.from_huggingface("<REPO_ID>")
assert dataset.records[0].suggestions is not None
```

**Checklist**

- [ ] I added relevant documentation
- [X] follows the style guidelines of this project
- [X] I did a self-review of my code
- [ ] I made corresponding changes to the documentation
- [X] 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)
- [X] I have added relevant notes to the CHANGELOG.md file (See
https://keepachangelog.com/)

---------

Co-authored-by: Gabriel Martin <[email protected]>

* docs: Update dataset page (argilla-io#3535)

<!-- 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

New page to showcase different workflows to update and make changes to
an existing Argilla dataset.

Closes argilla-io#3534

**Type of change**

(Remember to title the PR according to the type of change)

- [x] Documentation update

**How Has This Been Tested**

(Please describe the tests that you ran to verify your changes.)

- [x] `sphinx-autobuild` (read [Developer
Documentation](https://docs.argilla.io/en/latest/community/developer_docs.html#building-the-documentation)
for more details)

**Checklist**

- [ ] I added relevant documentation
- [x] I followed the style guidelines of this project
- [x] I did a self-review of my code
- [x] I made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I filled out [the contributor form](https://tally.so/r/n9XrxK)
(see text above)
- [x] I have added relevant notes to the `CHANGELOG.md` file (See
https://keepachangelog.com/)

---------

Co-authored-by: Alvaro Bartolome <[email protected]>

* docs: align docs with `FeedbackDataset` refactor using `tab-set`  (argilla-io#3531)

This PR adds the `tab-set` for the outdated code-blocks with the
upcoming release of Argilla v1.14.0 so as to showcase both the previous
and the new code-blocks for users that are still using Argilla v1.14.0
or lower.

Additionally, the style has been fixed in some documents while reviewing
the outdated code-blocks, some file paths references have been fixed to
point to the HTML files via relative paths, and some minor details.

Finally, a `warning` has also been included to let the users know that
Argilla v1.14.0 won't work for the moment with the
`ArgillaCallbackHandler` in `LangChain`.

**Type of change**

- [X] Documentation update

---------

Co-authored-by: Gabriel Martín Blázquez <[email protected]>
Co-authored-by: Natalia Elvira <[email protected]>

* fix: Argilla not working behind proxy (argilla-io#3543)

# Description

This PR updates the URL in which the Argilla App is mounted to be `"/"`,
as it's not required to change the URL of the server because the proxy
will know what rewrite has to be done.

In addition, the entrypoint scripts of both images have been updated to
add the `--root-path $ARGILLA_BASE_URL` option to the `uvicorn` command
if `ARGILLA_BASE_URL` env variable has been set. More info: [FastAPI -
Behind a
proxy](https://fastapi.tiangolo.com/advanced/behind-a-proxy/#behind-a-proxy).

Closes argilla-io#3542

**Type of change**

- [x] Bug fix (non-breaking change which fixes an issue)

**How Has This Been Tested**

- [x] `localhost/argilla` load in a web browser and can connect using
`rg.init` with an NGINX local setup:
  <details>
    <summary>Local Nginx</summary>
  
  `docker-compose.yaml`:
  
    ```yaml
  version: '3.8'
  
  services:
    argilla:
      image: argilla/argilla-quickstart:pr-3543
      environment:
        ARGILLA_BASE_URL: /argilla
        LOAD_DATASETS: none
      ports:
        - 6900:6900
  
    nginx:
      image: nginx:latest
      ports:
        - 80:80
      volumes:
        - ./nginx.conf:/etc/nginx/nginx.conf
    ```
    `nginx.conf`:
    ```
  events {}
  
  http {
    server {
        listen 80;
  
        server_name your_server_name_or_ip;
  
        location /argilla/ {
            proxy_pass http://argilla:6900/;
            proxy_set_header Host $host;
            proxy_set_header X-Real-IP $remote_addr;
            proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
            proxy_set_header X-Forwarded-Proto $scheme;
        }
    }
  }
    ```
  </details>

**Checklist**

- [x] I followed the style guidelines of this project
- [x] I did a self-review of my code
- [x] 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)
- [x] I have added relevant notes to the `CHANGELOG.md` file (See
https://keepachangelog.com/)

* fix: execute build docker only if build python (argilla-io#3547)

# Description

This PR updates the `package.yaml` workflow to **only** execute the
`build_server_docker_image` job if the `build_python_package` has been
executed and it's execution has succeeded (as the first job needs the
python package artifact generated by the second one)

Closes argilla-io#3544

**Type of change**

- [x] Bug fix (non-breaking change which fixes an issue)

**How Has This Been Tested**

N/A

**Checklist**

- [x] I followed the style guidelines of this project
- [x] I did a self-review of my code
- [x] 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/)

* chore: bump frontend version to `1.14.0`

* fix: `{FieldSchema, QuestionSchema}.name` attribute didn't have regex validation (argilla-io#3550)

# Description

This PR adds the regex validation to the `{FieldSchema,
QuestionSchema}.name` attribute. This regex validation is already
present in the server side, adding it to the client will allow to raise
a `ValidationError` before calling the server.

Closes argilla-io#3548

**Type of change**

- [x] Bug fix (non-breaking change which fixes an issue)

**How Has This Been Tested**

Added new unit tests covering the described issue above.

**Checklist**

- [x] I followed the style guidelines of this project
- [x] I did a self-review of my code
- [x] My changes generate no new warnings
- [x] 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)
- [x] I have added relevant notes to the `CHANGELOG.md` file (See
https://keepachangelog.com/)

---------

Co-authored-by: Alvaro Bartolome <[email protected]>

---------

Co-authored-by: gabrielmbmb <[email protected]>
Co-authored-by: Alvaro Bartolome <[email protected]>
Co-authored-by: Gabriel Martin <[email protected]>
Co-authored-by: Natalia Elvira <[email protected]>
Co-authored-by: Natalia Elvira <[email protected]>
artikandri added a commit to CLARIN-PL/argilla that referenced this pull request Oct 31, 2023
* chore: bump version to `1.14.0`

* chore: add `argilla-office-hours` Calendly link back

* fix: `push_to_argilla` to return `RemoteFeedbackDataset` without re-assigning class (argilla-io#3508)

# Description

This PR adds a `warnings.warn` message to let the users know that the
returned object from `FeedbackDataset.push_to_argilla` needs to be
handled, otherwise the dataset instance will remain local, as
`push_to_argilla` with no arguments won't do anything.

So on, before we had to `push_to_argilla` a new `FeedbackDataset` with
name and/or workspaces, and then we could `push_to_argilla` with no
arguments to push the updates, but we no longer need to do that, as now
we can re-use the returned `FeedbackDataset` from `push_to_argilla` to
have a `FeedbackDataset` fully integrated with Argilla.

```diff
import argilla as rg

dataset = rg.FeedbackDataset(...)
- dataset.push_to_argilla(name="my-dataset", workspace="my-workspace")
+ remote_dataset = dataset.push_to_argilla(name="my-dataset", workspace="my-workspace")

dataset.add_records(...)
- dataset.push_to_argilla()
```

**Type of change**

- [X] Bug fix (non-breaking change which fixes an issue)
- [X] Breaking change (fix or feature that would cause existing
functionality to not work as expected)

**How Has This Been Tested**

- [X] Catch returned object in `push_to_argilla` and handle
`DeprecationWarning`

**Checklist**

- [ ] I added relevant documentation
- [X] follows the style guidelines of this project
- [X] I did a self-review of my code
- [ ] I made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [X] 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/)

---------



* feat: add `delete` method to `FeedbackDataset` in Argilla (argilla-io#3512)

# Description

This PR adds a `delete` method for `RemoteFeedbackDataset` which is a
`FeedbackDataset` that has been pushed to Argilla. So on, the `delete`
method deletes a dataset in Argilla, but it's just available for `owner`
users and for `admin` users with that dataset within their workspace,
otherwise the method won't work and will raise a `PermissionError`.

So on, now both `owner` and `admin` users can delete a `FeedbackDataset`
from Argilla as:

```python
import argilla as rg

rg.init(...)

dataset = FeedbackDataset.from_argilla(...)
dataset.delete()
```

Or alternatively

```python
import argilla as rg

rg.init(...)

dataset = FeedbackDataset(...)
remote_dataset = dataset.push_to_argilla(...)
remote_dataset.delete()
```

Closes argilla-io#3413

**Type of change**

- [X] New feature (non-breaking change which adds functionality)

**How Has This Been Tested**

- [X] Add integration tests for `RemoteFeedbackDataset.delete` including
every role

**Checklist**

- [ ] I added relevant documentation
- [X] follows the style guidelines of this project
- [X] I did a self-review of my code
- [ ] I made corresponding changes to the documentation
- [X] My changes generate no new warnings
- [X] 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)
- [X] I have added relevant notes to the CHANGELOG.md file (See
https://keepachangelog.com/)

* fix: `publish_dataset` to check `required=True` on at least one field/question (argilla-io#3511)

# Description

This PR fixes a bug with the `PUT /api/v1/datasets/{dataset_id}/publish`
endpoint, as there was a missing check on the `required` flag for each
field/question which was allowing to publish `FeedbackTask` datasets
with no required fields and/or questions.

**Type of change**

- [X] Bug fix (non-breaking change which fixes an issue)

**How Has This Been Tested**

- [X] Update existing unit/integration tests
- [X] Add unit/integration tests with `required=True` and
`required=False`

**Checklist**

- [ ] I added relevant documentation
- [X] follows the style guidelines of this project
- [X] I did a self-review of my code
- [ ] I made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [X] 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/)

---------



* docs: fix link to ASGI middleware tutorial (argilla-io#3518)

# Description

Fix broken link to ASGI middleware tutorial.

**Type of change**

- [x] Documentation update

**How Has This Been Tested**

N/A

**Checklist**

- [ ] I added relevant documentation
- [x] follows the style guidelines of this project
- [x] I did a self-review of my code
- [ ] I made corresponding changes to the documentation
- [x] 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/)

* fix: `thread_ts` is not always present (argilla-io#3538)

# Description

The `slack-post-credentials` action is failing from time to time because
not all the messages have the `thread_ts` key. This PR fixes this issue
using the `ts` key instead of the `thread_ts`.

**Type of change**

- [x] Bug fix (non-breaking change which fixes an issue)

**How Has This Been Tested**

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

The deployment job got executed fine for this PR:
https://github.com/argilla-io/argilla/actions/runs/5818199084/job/15774977297?pr=3538

**Checklist**

- [x] I followed the style guidelines of this project
- [x] I did a self-review of my code
- [x] 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/)

* fix: restore `suggestions` and `responses` as rows in `HuggingFaceDatasetMixin` (argilla-io#3539)

# Description

This PR addresses the feature mentioned by @tomaarsen at
argilla-io#3467, to basically export the
`responses` for the existing questions in the `FeedbackDataset` when
calling `push_to_huggingface` in a row-based format instead of using the
`Sequence` from 🤗`datasets`. This makes the dataset from the HuggingFace
Hub more readable, and also easier to use with other frameworks and/or
libraries.

```diff
- {"user_id": ["A", "B"], "value": [1, 2], "status": ["C", "D"]}
+ [{"user_id": "A", "value": 1, "status": "C"}, {"user_id": "B", "value": 2, "status": "D"}]
```

Additionally, this PR also ensure that the backwards compatibility is
preserved with the previous versions, and assumes the new format as the
default one when calling `format_as("datasets")`.

Finally, this PR also solves an issue reported by @nataliaElv recently
that was affecting the `suggestions` when calling
`FeedbackDataset.from_argilla`, as those were just kept when there were
`responses`, otherwise, a `continue` statement was being called so the
`suggestions` were completely ignored.

**Type of change**

- [X] Bug fix (non-breaking change which fixes an issue)
- [X] New feature (non-breaking change which adds functionality)

**How Has This Been Tested**

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

- [X] Ran the following script and tested different combinations as the
connection to HuggingFace is mocked, but we should create a fake/testing
user at some point to avoid overloading `argilla-io`

```python
import argilla as rg

dataset = rg.FeedbackDataset(
    fields=[
        rg.TextField(
            name="prompt",
            required=True,
        ),
    ],
    questions=[
        rg.TextQuestion(
            name="response-edit",
            title="Add or edit the response if necessary",
            required=True,
        ),
    ],
)
dataset.add_records(
    rg.FeedbackRecord(
        fields={
            "prompt": "This is the prompt!",
        },
        suggestions=[
            {
                "question_name": "response-edit",
                "value": "This is the suggestion!"
            }
        ],
    )
)
dataset.push_to_huggingface("<REPO_ID>")
dataset = rg.FeedbackDataset.from_huggingface("<REPO_ID>")
assert dataset.records[0].suggestions is not None
```

**Checklist**

- [ ] I added relevant documentation
- [X] follows the style guidelines of this project
- [X] I did a self-review of my code
- [ ] I made corresponding changes to the documentation
- [X] 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)
- [X] I have added relevant notes to the CHANGELOG.md file (See
https://keepachangelog.com/)

---------



* docs: Update dataset page (argilla-io#3535)

<!-- 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

New page to showcase different workflows to update and make changes to
an existing Argilla dataset.

Closes argilla-io#3534

**Type of change**

(Remember to title the PR according to the type of change)

- [x] Documentation update

**How Has This Been Tested**

(Please describe the tests that you ran to verify your changes.)

- [x] `sphinx-autobuild` (read [Developer
Documentation](https://docs.argilla.io/en/latest/community/developer_docs.html#building-the-documentation)
for more details)

**Checklist**

- [ ] I added relevant documentation
- [x] I followed the style guidelines of this project
- [x] I did a self-review of my code
- [x] I made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I filled out [the contributor form](https://tally.so/r/n9XrxK)
(see text above)
- [x] I have added relevant notes to the `CHANGELOG.md` file (See
https://keepachangelog.com/)

---------



* docs: align docs with `FeedbackDataset` refactor using `tab-set`  (argilla-io#3531)

This PR adds the `tab-set` for the outdated code-blocks with the
upcoming release of Argilla v1.14.0 so as to showcase both the previous
and the new code-blocks for users that are still using Argilla v1.14.0
or lower.

Additionally, the style has been fixed in some documents while reviewing
the outdated code-blocks, some file paths references have been fixed to
point to the HTML files via relative paths, and some minor details.

Finally, a `warning` has also been included to let the users know that
Argilla v1.14.0 won't work for the moment with the
`ArgillaCallbackHandler` in `LangChain`.

**Type of change**

- [X] Documentation update

---------




* fix: Argilla not working behind proxy (argilla-io#3543)

# Description

This PR updates the URL in which the Argilla App is mounted to be `"/"`,
as it's not required to change the URL of the server because the proxy
will know what rewrite has to be done.

In addition, the entrypoint scripts of both images have been updated to
add the `--root-path $ARGILLA_BASE_URL` option to the `uvicorn` command
if `ARGILLA_BASE_URL` env variable has been set. More info: [FastAPI -
Behind a
proxy](https://fastapi.tiangolo.com/advanced/behind-a-proxy/#behind-a-proxy).

Closes argilla-io#3542

**Type of change**

- [x] Bug fix (non-breaking change which fixes an issue)

**How Has This Been Tested**

- [x] `localhost/argilla` load in a web browser and can connect using
`rg.init` with an NGINX local setup:
  <details>
    <summary>Local Nginx</summary>
  
  `docker-compose.yaml`:
  
    ```yaml
  version: '3.8'
  
  services:
    argilla:
      image: argilla/argilla-quickstart:pr-3543
      environment:
        ARGILLA_BASE_URL: /argilla
        LOAD_DATASETS: none
      ports:
        - 6900:6900
  
    nginx:
      image: nginx:latest
      ports:
        - 80:80
      volumes:
        - ./nginx.conf:/etc/nginx/nginx.conf
    ```
    `nginx.conf`:
    ```
  events {}
  
  http {
    server {
        listen 80;
  
        server_name your_server_name_or_ip;
  
        location /argilla/ {
            proxy_pass http://argilla:6900/;
            proxy_set_header Host $host;
            proxy_set_header X-Real-IP $remote_addr;
            proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
            proxy_set_header X-Forwarded-Proto $scheme;
        }
    }
  }
    ```
  </details>

**Checklist**

- [x] I followed the style guidelines of this project
- [x] I did a self-review of my code
- [x] 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)
- [x] I have added relevant notes to the `CHANGELOG.md` file (See
https://keepachangelog.com/)

* fix: execute build docker only if build python (argilla-io#3547)

# Description

This PR updates the `package.yaml` workflow to **only** execute the
`build_server_docker_image` job if the `build_python_package` has been
executed and it's execution has succeeded (as the first job needs the
python package artifact generated by the second one)

Closes argilla-io#3544

**Type of change**

- [x] Bug fix (non-breaking change which fixes an issue)

**How Has This Been Tested**

N/A

**Checklist**

- [x] I followed the style guidelines of this project
- [x] I did a self-review of my code
- [x] 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/)

* chore: bump frontend version to `1.14.0`

* fix: `{FieldSchema, QuestionSchema}.name` attribute didn't have regex validation (argilla-io#3550)

# Description

This PR adds the regex validation to the `{FieldSchema,
QuestionSchema}.name` attribute. This regex validation is already
present in the server side, adding it to the client will allow to raise
a `ValidationError` before calling the server.

Closes argilla-io#3548

**Type of change**

- [x] Bug fix (non-breaking change which fixes an issue)

**How Has This Been Tested**

Added new unit tests covering the described issue above.

**Checklist**

- [x] I followed the style guidelines of this project
- [x] I did a self-review of my code
- [x] My changes generate no new warnings
- [x] 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)
- [x] I have added relevant notes to the `CHANGELOG.md` file (See
https://keepachangelog.com/)

---------



---------

Co-authored-by: gabrielmbmb <[email protected]>
Co-authored-by: Alvaro Bartolome <[email protected]>
Co-authored-by: Gabriel Martin <[email protected]>
Co-authored-by: Natalia Elvira <[email protected]>
Co-authored-by: Natalia Elvira <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: integration Indicates integrations with third parties
Projects
None yet
5 participants