feat(double-check): Allow validation of imported annotations [ADEV-10] #191

ElodieENSTA · 2024-03-19T15:03:24Z

New styled form components for APLOSE
Update of the campaign creation form
- New organisation of the fields
- Remove files distribution select (random/sequential) in favor of a constent "sequential" distribution
- Remove annotation scope select (box/whole files) in favor of a constent "whole file" mode
- New annotation mode select (create annotations / check annotations). The "create annotation" is the common, existing, one ; The "check annotations" mode allows to import a resuts.csv file in order to validate them in the annotator page
Update the style of the edit campaign form
Update of the annotator to take in account the new "Check annotations" mode. In this mode:
- annotations cannot be created, edited or deleted
- annotations can be validated or invalidated. By default, all annotations are considered as valid

Automatic testing:

The new form components have their component tests
E2E tests:
- Update of the campaign creation tests
- New update campaign test

…ecated create-react-app (create / move / delete files)

…renamed files)

…n following commits to try to keep history)

…outer-dom

…o make them work

…pt __WIP__

Redirect login already logged

rvovard

As I'm afraid to lose my comments: first part of the review (110 files viewed over 186).
Really good work, many things are cleaner than before! 👍
Most of my comments are questions or remarks, some things can be done later, don't feel that you have to change everything in this PR 🙂

backend/api/models/datasets.py

backend/api/views/annotation_task.py

backend/tests/views/annotation_campaign.py

backend/tests/views/annotation_task.py

frontend/src/App.tsx

rvovard · 2024-04-14T18:29:05Z

frontend/index.html

+    <link rel="preconnect" href="https://fonts.googleapis.com">
+    <link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
+    <link href="https://fonts.googleapis.com/css2?family=Exo+2&display=swap" rel="stylesheet">


I think it's better to download the fonts you want to use and to include them in our fonts directory.
We don't want to depend on another website (which can be down or slow, even if it's Google), and people who don't want to be tracked by Google (fonts come with associated cookies) might filter Google CDN.

(Though, the "Roboto" folder might be cleaned, I don't think we need all those versions of the font)

I understand, I'll will add it to the repo.

For "Roboto" I didn't find any use of this font files (in @font-face) so maybe all of it should be removed?

rvovard · 2024-04-14T18:38:05Z

frontend/src/components/form/inputs/chips-input.tsx

+  return <div id="aplose-input" className="chips" aria-disabled={ disabled }>
+    { label && <p id="label"
+                  className={ required ? 'required' : '' }>
+      { label }{ required ? '*' : '' }
+    </p> }


General comments as it applies to several files:

You used id attribute which is supposed to appear only one time on a HTML document. As inputs can be used many times, I think the page won't be syntactically correct.

Why don't use label tag instead of p? It's semantically better.

I understand your concern about id attribute. I'll update it over time.

I'm not used to the label, that's why i didn't though of it. I'll change it.

frontend/src/components/form/inputs/select.tsx

frontend/src/components/form/searchbar/searchbar.component.tsx

- include Exo 2 font files - clean commented code - Use appropriate html tags instead of generic ones

rvovard

End of the review, but very few things to add 🙂
Once again, well done!

rvovard · 2024-04-16T19:42:31Z

frontend/src/services/create-campaign/import-annotations.ts

+            })
+          ));
+        } catch (e) {
+          console.debug('unrecognized file', e)


You used console.warn earlier in the file, I think we can use it too here?

For sure, and even better I should send the exact error to the view. I used it to debug the code but the original error could be useful for users too, to understand what went wrong precisely (ticket created for that)

frontend/src/services/utils/format.tsx

frontend/src/slices/annotator/annotations.ts

rvovard · 2024-04-16T20:05:12Z

frontend/src/types/user.ts

+// export class User {
+//   id: number;
+//   username: string;
+//   email: string;
+//   first_name: string;
+//   last_name: string;
+//
+//   constructor(data: { id: number, username: string, email: string, first_name: string, last_name: string }) {
+//     this.id = data.id;
+//     this.username = data.username;
+//     this.email = data.email;
+//     this.first_name = data.first_name;
+//     this.last_name = data.last_name;
+//   }
+//
+//   get display_name(): string {
+//     if (this.first_name && this.last_name) return `${ this.first_name } ${ this.last_name }`;
+//     else return this.username;
+//   }
+// }


You know the drill: is this still useful?

Thanks for the remind ^^
I removed the interface to use this class. Globally, I'd like to replace a lot of interfaces that reprensent API data by object to simplify its use and common data transformation used around the app.

frontend/src/view/audio-annotator/audio-annotator.page.tsx

frontend/tsconfig.json

…k campaign [ADEV-100] Add detector name in detections list bloc (annotator)

…her than filename

request-checks: true

…and add force create route

# Conflicts: # backend/api/admin/__init__.py # frontend/index.html # frontend/package-lock.json # frontend/package.json # frontend/src/App.tsx # frontend/src/css/ionic-override.css # frontend/src/fonts/Exo_2/OFL.txt # frontend/src/fonts/Exo_2/README.txt # frontend/src/view/annotation-campaign-update/edit-annotation-campaign.page.tsx # frontend/src/view/audio-annotator/components/bloc/confidence-indicator-bloc.component.tsx # frontend/src/view/login.page.tsx # frontend/vite.config.ts

rvovard and others added 30 commits February 1, 2024 11:36

chore(frontend): initialize aplose frontend with Vite instead of depr…

acbf1aa

…ecated create-react-app (create / move / delete files)

chore(frontend): initialize aplose frontend with Vite - part 2 (edit …

2311abc

…renamed files)

chore(frontend): move files to use TypeScript (editing will be done i…

d6bf727

…n following commits to try to keep history)

chore(frontend): update main.tsx for ViteJS and add forgotten react-r…

47a1a61

…outer-dom

chore(vitejs): update all component files with no big modifications t…

7841cf9

…o make them work

chore(frontend): update a new batch of components for typescript

009cb2b

chore(frontend): __WIP__ update AudioAnnotator component for typescri…

2cdb599

…pt __WIP__

App & Auth

18a8ec5

DatasetList

faf1410

Annotation campaign list

1c7b7e2

Annotation campaign detail

c21d100

Annotation task list

746e7e5

CreateAnnotationCampaign.tsx

b22597b

EditAnnotationCampaign.tsx

4ecacb1

AudioAnnotator.tsx

048615c

fix DatasetList.tsx

a5e0ebf

fix wip

4d79fea

Cypress cherry pick

f7739a9

Datasets tests updated

1aa3cbd

Redirect login already logged

AnnotationCampaignDetail.tsx

90db1db

wip - annotator

29d2db7

wip - auth

556aba8

wip - auth

3902966

wip - global components

b066fcd

wip - global components

e0e635b

wip - global components

a9b3454

wip - login page

88115d2

wip - dataset list page

a691162

wip - annotation campaign list page

879694d

wip - annotation campaign detail page

6ce867d

ElodieENSTA added 5 commits April 12, 2024 09:43

FIX creating multiple detectors with same config

3862e4c

FIX: file upload on firefox

35c0b51

FIX create issue with confidence set

1295b9e

fix import

f149a9b

fix css

b90601f

rvovard requested changes Apr 14, 2024

View reviewed changes

ElodieENSTA added 3 commits April 15, 2024 13:41

Remove useless TODO

da876b0

Clean after review

ab5c6e6

- include Exo 2 font files - clean commented code - Use appropriate html tags instead of generic ones

Fix import issues

40605f5

rvovard requested changes Apr 16, 2024

View reviewed changes

ElodieENSTA added 18 commits April 17, 2024 16:56

[ADEV-95] Handle task without annotations to check

eac483c

[ADEV-96] Replace "annotations" by "detections" in annotator for chec…

3d90e4a

…k campaign [ADEV-100] Add detector name in detections list bloc (annotator)

[ADEV-103] Show CSV import errors in view

e6d0e2e

[ADEV-93] Improve Detector form on CSV import

659f9d6

[ADEV-111] Import double-check annotations based on the timestamp rat…

9c4dcd0

…her than filename

Fix test after review

4ce8a61

Removed "campaign_user_list" tests after review

0613447

Update User interface to class after review

7227442

Update package.json

0c409a8

request-checks: true

[ADEV-111] Add error for results where back cannot associate files ; …

4f8b85b

…and add force create route

[ADEV-117] Add results counts on task list

7bb4ff6

Add loading behavior at campaign creation submit

3f19ee9

[ADEV-117] Fix: tasks request must filter annotator

01eee4f

Merge remote-tracking branch 'origin/master' into feature/double_check

6f935f5

Fix issue with required input next to select

e854379

Small fixes: nav, style, and show full boxes as tags

97bd475

FIX: annotation import and small other issues

411d9f5

ElodieENSTA merged commit 7cf0a87 into master Jun 24, 2024
3 checks passed

ElodieENSTA deleted the feature/double_check branch June 24, 2024 08:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(double-check): Allow validation of imported annotations [ADEV-10] #191

feat(double-check): Allow validation of imported annotations [ADEV-10] #191

ElodieENSTA commented Mar 19, 2024

rvovard left a comment

rvovard Apr 14, 2024

ElodieENSTA Apr 15, 2024

rvovard Apr 14, 2024

ElodieENSTA Apr 15, 2024

rvovard left a comment

rvovard Apr 16, 2024

ElodieENSTA Apr 17, 2024

rvovard Apr 16, 2024

ElodieENSTA Apr 30, 2024

feat(double-check): Allow validation of imported annotations [ADEV-10] #191

feat(double-check): Allow validation of imported annotations [ADEV-10] #191

Conversation

ElodieENSTA commented Mar 19, 2024

rvovard left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rvovard left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment