-
Notifications
You must be signed in to change notification settings - Fork 412
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: add validation layer for metadata_properties
and metadata_filters
#3860
feat: add validation layer for metadata_properties
and metadata_filters
#3860
Conversation
…o feat/add-metadata-validation
Because the `metadata` is always optional, so it will just be validated if provided
FYI @frascuchon I had to revert the |
Codecov ReportAttention:
... and 4 files with indirect coverage changes 📢 Thoughts on this report? Let us know!. |
The URL of the deployed environment for this PR is https://argilla-quickstart-pr-3860-ki24f765kq-no.a.run.app |
b3e0015
to
a0e1c0a
Compare
In favour of `@property` + `@abstractmethod`. More information at https://docs.python.org/3.11/library/abc.html#abc.abstractproperty
37eb163
into
feature/support-for-metadata-filtering-and-sorting
Description
This PR adds a validation layer for both the
metadata_properties
whenmetadata
is provided viaFeedbackRecord
when adding that record to either a local or existingFeedbackDataset
, as well as validating themetadata_filters
provided to thefilter_by
method forRemoteFeedbackDataset
.These validation layers would help reduce the API workload as the validation will be done client-side, which implies that the client will handle the validation first.
In this case, as we have the following properties:
TermsMetadataProperty
,IntegerMetadataProperty
andFloatMetadataProperty
; we'll need to ensure that the values provided for those are correct, both onadd_records
and also onfilter_by
.The approach is similar to the one already implemented for the
fields
of aFeedbackRecord
, but extending it to themetadata_properties
to also include a validation layer that ensures not just the type of the value provided, but whether it's compliant with the pre-defined values forTermMetadataProperty
or the pre-defined boundaries for bothIntegerMetadataProperty
andFloatMetadataProperty
. The core difference is that besides thepydantic
validation, there's also a method in each property that validates the values provided in the filters related to those metadata properties.Closes #3859
Type of change
How Has This Been Tested
generate_pydantic_schema_for_metadata
method_validate_filter
and_pydantic_field_with_validator
in each property i.e.TermsMetadataProperty
,IntegerMetadataProperty
, andFloatMetadataProperty
add_records
with both correct and wrongmetadata
add_records
andfilter_by
to ensure that the validation is correct both before and after applying those opsChecklist