-
Notifications
You must be signed in to change notification settings - Fork 399
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 basic support to bool features #5576
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From my understanding, this slackens metadata typing to Any
. That makes sense for current changes to the direction.
@frascuchon We probably now have a lot of redundant testing and docs?
This is an SDK change only. Not sure to understand your question completely |
@frascuchon should we include tests here: argilla/tests/unit/test_settings/test_metadata.py |
Not really, we are just changing the metadata content, not the settings definition. But it makes sense to change the settings to accept not only str values. I will create a separate PR adding the change on the server side. |
…terms metadata (#5589) # 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. --> This PR changes the terms metadata configuration to accept other values than str. This allows users to define terms metadata based on numeric or boolean values. **Type of change** <!-- Please delete options that are not relevant. Remember to title the PR according to the type of change --> - Refactor (change restructuring the codebase without changing functionality) - Improvement (change adding some improvement to an existing functionality) **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/)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #5576 +/- ##
========================================
Coverage 91.22% 91.22%
========================================
Files 145 145
Lines 6030 6030
========================================
Hits 5501 5501
Misses 529 529
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Description
Also, relax type casting for metadata values, accepting Any.
Type of change
How Has This Been Tested
Checklist