fix: push_to_argilla
and from_argilla
consistency issues
#3234
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR removes the unnecessary
json.loads(field.json())
andjson.loads(question.json())
as all the fields in thosepydantic.BaseModel
s are JSON-serializable, so we can just provide thedict
to the endpoint.Besides that, we're also using the
dict
forFeedbackRecord
s, and then ensuring that theuser_id
in each response, which is anUUID
and so on, not JSON-serializable, is properly serialized as a string in theadd_records
SDK function.This also aligns both the
FeedbackRecord
logging in Argilla under the scenarios of pushing a new dataset and pushing records to an existing dataset.Finally, also the
from_argilla
field and question re-construction is patched, as we were still usingconstruct
which was not respecting theConfig
set on thosepydantic.BaseModel
s asconstruct
impliesConfig.extra = 'allow'
. So on, now that's patched too, and theFeedbackDataset
retrieved viafrom_argilla
is exactly the same one as the one logged viapush_to_argilla
and/orpush_to_huggingface
; as well as theargilla_id
assignment when retrieving aFeedbackDataset.from_argilla
once theargilla_id
has already been set ascls.argilla_id = ...
.Type of change
How Has This Been Tested
push_to_argilla
to make sure the API doesn't throw any HTTP 422 when the payload contains UUIDsfrom_argilla
to make sure the retrieveFeedbackDataset
is consistent to what we should expectChecklist