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(Client): Add a new component for multi-label text classification bulk annotation #2291

Conversation

keithCuniah
Copy link
Contributor

@keithCuniah keithCuniah commented Feb 3, 2023

Description

Components :

  • TextClassificationBulkAnnotation
  • BulkAnnotation
  • BulkAnnotationForm

Note :

  • I removed the remove attribute from the form since we can guess which annotation will be remove only with the selected attribute, I will update the doc
  • refactoring for later : it could be a nice to have a builder design pattern to manage forms

What is left :

  • use Vuelidate plugin to manage disable enable submit and cancel buttons (I didn't use vuelidate, since only one computed was enough)
  • disable state of the dropdown button
  • implement the search in the BulkAnnotation component ( bug to correct : need to keep the previous selection after search )
  • Style
  • unit test
  • update documentation

Closes #2267

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)

  • TextClassification multilabel

Checklist

  • I have merged the original branch into my forked branch
  • I added relevant documentation
  • follows the style guidelines of this project
  • I did a self-review of my code
  • I added comments to 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

@keithCuniah keithCuniah changed the title feat(Client): components TextClassificationBulkAnnotation, BulkAnnota… feat(Client): Add a new component for multi-label text classification bulk annotation Feb 3, 2023
…omponent-for-multilabel-textclassification-bulk-annotation
@keithCuniah keithCuniah marked this pull request as ready for review February 4, 2023 03:14
@leiyre
Copy link
Member

leiyre commented Feb 6, 2023

One comment, in the case of a label already applied to some records, the icon of the cross and the label have different behavior.
Let's imagine that we have POSITIVE (2), in this case the cross must be visible, when we click on the cross we have to delete the annotation, when we click on the label we have to apply the label to all the selected records. The number of records applied should change, if we click on the cross, we remove the record number and if we apply the label we need to show the number of all selected records.

If we have 20 records selected in a page:

X POSITIVE (2): If I click the cross existing labels must be deleted. Label looks like => POSITIVE
X POSITIVE (2): If I click the label existing labels must be applied to all records. Label looks like => X POSITIVE (20)

@keithCuniah keithCuniah force-pushed the feature/new-component-for-multilabel-textclassification-bulk-annotation branch from 055c9a5 to b129028 Compare February 7, 2023 10:51
@frascuchon
Copy link
Member

I've been doing some testing in dev and I've seen some weird behavior for single-label:

1. Discard a validated record
Screenshot 2023-02-07 at 13 23 17

Screenshot 2023-02-07 at 13 23 22

2. Enabling by clicking the selected label let the record empty, and no pending status is shown:
Screenshot 2023-02-07 at 13 23 30

cc @keithCuniah @leiyre

@keithCuniah keithCuniah force-pushed the feature/new-component-for-multilabel-textclassification-bulk-annotation branch from 790a164 to cace7eb Compare February 7, 2023 14:21
@leiyre
Copy link
Member

leiyre commented Feb 9, 2023

@frascuchon your example for me is the correct behavior, we emit a reset in case of single label with empty labels (it's not new behavior and we can discuss). I see some things that don't fit, there is supposed to be no pending status in the single Text Classifier Classifier, so for me the "clear" button doesn't make sense for this case. I will comment in specs.

# Description
Add feedback on discard/validation for bulk annotation. This concerned
the 3 tasks

ref #2291 

**Type of change**

- [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] TextClassification (single and multilabel)
- [x] TokenClassification 
- [x] Text2Text

**Checklist**

- [ ] I have merged the original branch into my forked branch
- N/A I added relevant documentation
- [x] follows the style guidelines of this project
- [x] I did a self-review of my code
- N/A I added comments to my code
- [N/A I made corresponding changes to the documentation
- [x] My changes generate no new warnings
- N/A I have added tests that prove my fix is effective or that my
feature works

---------

Co-authored-by: keithCuniah <[email protected]>
@keithCuniah keithCuniah merged commit b08faa9 into feature/bulk-annotation-improvement Feb 14, 2023
@keithCuniah keithCuniah deleted the feature/new-component-for-multilabel-textclassification-bulk-annotation branch February 14, 2023 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a new component for multi-label text classification bulk annotation
3 participants