Skip to content
This repository has been archived by the owner on Jun 14, 2024. It is now read-only.

feat: create span question responses #46

Merged

Conversation

jfcalvo
Copy link
Member

@jfcalvo jfcalvo commented Feb 29, 2024

Description

This PR add changes so we can create responses for span questions.

Closes #45

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)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (change restructuring the codebase without changing functionality)
  • Improvement (change adding some improvement to an existing functionality)
  • Documentation update

How Has This Been Tested

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

  • Adding new tests.

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

@jfcalvo jfcalvo requested a review from frascuchon February 29, 2024 12:23
Copy link

codecov bot commented Mar 1, 2024

Codecov Report

Attention: Patch coverage is 99.39759% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 90.32%. Comparing base (c007e80) to head (6541a64).
Report is 11 commits behind head on feat/support-for-span-questions.

Files Patch % Lines
src/argilla_server/models/questions.py 97.77% 1 Missing ⚠️
Additional details and impacted files
@@                         Coverage Diff                         @@
##           feat/support-for-span-questions      #46      +/-   ##
===================================================================
+ Coverage                            90.26%   90.32%   +0.06%     
===================================================================
  Files                                  187      188       +1     
  Lines                                 9210     9283      +73     
===================================================================
+ Hits                                  8313     8385      +72     
- Misses                                 897      898       +1     

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


class SpanQuestionSettings(BaseQuestionSettings):
type: Literal[QuestionType.span]
options: List[ValueTextQuestionSettingsOption]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
options: List[ValueTextQuestionSettingsOption]
options: List[ValueTextQuestionSettingsOption]
field: str # <- ??

Copy link
Member Author

@jfcalvo jfcalvo Mar 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not using it (right now) to check responses that's why I didn't add it. Additionally the handler schemas are already checking that it's there and we are validating that the field is valid in the question creation.

@jfcalvo jfcalvo changed the title [WIP] feat: create span question responses feat: create span question responses Mar 5, 2024
frascuchon and others added 5 commits March 5, 2024 14:01
# Description

This PR add changes so span questions can be updated and labels on
settings reorder. The labels can not be changed only reordered as we are
already doing for label and multi label questions.

Closes #49 

**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)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] Refactor (change restructuring the codebase without changing
functionality)
- [ ] Improvement (change adding some improvement to an existing
functionality)
- [ ] Documentation update

**How Has This Been Tested**

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

- [ ] Test A
- [ ] Test B

**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](https://tally.so/r/n9XrxK)
(see text above)
- [ ] I have added relevant notes to the CHANGELOG.md file (See
https://keepachangelog.com/)
<!-- 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

This PR review the record creation/modification and splits the
creation/modification for each of its relationships (responses,
suggestions and vectors)

Also, the response validation function expects a record as an argument
instead of a dataset (the dataset must be available from the record)


Closes #<issue_number>

**Type of change**

(Please delete options that are not relevant. Remember to title the PR
according to the type of change)

- [ ] New feature (non-breaking change which adds functionality)
- [X] Refactor (change restructuring the codebase without changing
functionality)
- [ ] Improvement (change adding some improvement to an existing
functionality)

**How Has This Been Tested**

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

- [ ] Test A
- [ ] Test B

**Checklist**

- [ ] 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
- [ ] 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/)

Co-authored-by: José Francisco Calvo <josefranciscocalvo@gmail.com>
)

# Description

This PR add additional validations for span responses and suggestions:
* Check that the required `field` to be annotated by the response or
suggestion is present in the associated record.
* Check that annotation `start` attribute is lower than the length in
characters of the associated record field.
* Check that annotation `end` attribute is lower or equal than the
length in characters of the associated record field.

These validations are done in the `check_response` function present in
`questions` models package.

Refs #45

**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)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] Refactor (change restructuring the codebase without changing
functionality)
- [ ] Improvement (change adding some improvement to an existing
functionality)
- [ ] Documentation update

**How Has This Been Tested**

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

- [x] Adding new tests.

**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](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: Paco Aranda <francis@argilla.io>
# Description

Add `inserted_at` and `updated_at` fields to `Suggestion` schema for API
v1.

Refs #45 

**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)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] Refactor (change restructuring the codebase without changing
functionality)
- [ ] Improvement (change adding some improvement to an existing
functionality)
- [ ] Documentation update

**How Has This Been Tested**

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

- [x] Modifying tests.

**Checklist**

- [ ] I added relevant documentation
- [ ] 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
- [ ] 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: Paco Aranda <francis@argilla.io>
@frascuchon frascuchon self-requested a review March 6, 2024 15:02
@jfcalvo jfcalvo merged commit 66cccce into feat/support-for-span-questions Mar 6, 2024
13 checks passed
@jfcalvo jfcalvo deleted the feat/create-span-question-responses branch March 6, 2024 15:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants