-
Notifications
You must be signed in to change notification settings - Fork 3.1k
feat: Create proper results for CustomInterface #8547
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
Conversation
✅ Deploy Preview for label-studio-docs-new-theme canceled.
|
✅ Deploy Preview for label-studio-playground ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for heartex-docs canceled.
|
✅ Deploy Preview for label-studio-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #8547 +/- ##
===========================================
- Coverage 67.46% 58.06% -9.41%
===========================================
Files 780 560 -220
Lines 59861 39025 -20836
Branches 10136 10356 +220
===========================================
- Hits 40387 22660 -17727
+ Misses 19471 16362 -3109
Partials 3 3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
/git merge
|
try { | ||
const areaValue = { custominterface: value }; | ||
const resultValue = { custominterface: value }; | ||
self.annotation.createResult(areaValue, resultValue, self, self.toname); |
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.
why did you remove the main call creating result and region?
if (store && store.events) { | ||
store.events.invoke("updateAnnotation", store, annotation).catch(() => {}); | ||
if (store && store.events && typeof store.events.invoke === "function") { | ||
try { | ||
const res = store.events.invoke("updateAnnotation", store, annotation); | ||
if (res && typeof res.catch === "function") res.catch(() => {}); | ||
} catch (e) {} | ||
} | ||
} else { | ||
if (store && store.events) { | ||
store.events.invoke("submitAnnotation", store, annotation).catch(() => {}); | ||
if (store && store.events && typeof store.events.invoke === "function") { | ||
try { | ||
const res = store.events.invoke("submitAnnotation", store, annotation); | ||
if (res && typeof res.catch === "function") res.catch(() => {}); | ||
} catch (e) {} |
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.
we have to remove all these calls completely! result update should not submit annotation, that's totally wrong. also it looks like it doesn't work anyway.
detector: (sn: any) => Boolean(sn?.value?.custominterface || sn?.custominterface), | ||
// Do not detect regions by value for CustomInterface. Classification areas (global results) | ||
// should deserialize as ClassificationArea, not CustomRegionModel, otherwise addResult is missing. | ||
detector: () => false, |
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.
if there are problems with false detection of classification we should fix the problem, not break the detection completely.
- Proper relations to other parts of LSF like `object` and types - Remove some excess functions and fields breaking the LSF - Add missing AreaMixin
05ac768
to
d18bc59
Compare
We should not call annotation submission methods from the tag, it's totally wrong.
- Add missing serialize() to save results properly - Add `update(value)` and `get value` for better region api - Fix tag type: not a control, not a classification; we should use ObjectBase - Remove more useless code - Use `observer()` for rendered tag so it reacts on region changes - Also fix dependency to react on regions
That allows to use usual JS code with &, <, etc.
First try