-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Incremental save of annotation. #120
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.
++: /* exported createExportContainer ExportType */
Also need add these symbols into eslintrc.conf.js global variables
@@ -3150,6 +3147,25 @@ class PointsView extends PolyShapeView { | |||
} | |||
} | |||
|
|||
function createExportContainer() { |
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.
I believe it is better to move this function into base.js
The same for ExportType object
cvat/apps/engine/annotation.py
Outdated
@@ -970,13 +1090,13 @@ def _save_paths_to_db(self): | |||
# for Postgres bulk_create will return objects with ids even ids | |||
# are auto incremented. Thus we will not be inside the 'if'. | |||
if shape_type == 'polygon_paths': | |||
db_paths = list(self.db_job.objectpath_set.filter(shapes="polygons")) | |||
db_paths = list(self.db_job.objectpath_set.exclude(id__in=db_paths_ids).filter(shapes="polygons")) |
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 we use exclude() with indexes which have existed before bulk_create(), we need only one row:
db_paths = list(self.db_job.objectpath_set.exclude(id__in=db_paths_ids)
Right?
cvat/apps/engine/models.py
Outdated
@@ -145,6 +145,7 @@ class Meta: | |||
class Shape(models.Model): | |||
occluded = models.BooleanField(default=False) | |||
z_order = models.IntegerField(default=0) | |||
client_id = models.IntegerField(default=-1) |
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.
Probably better add client_id into Annotation class instead of Shape and ObjectPath
cvat/apps/engine/annotation.py
Outdated
self._save_shapes_to_db() | ||
self._save_paths_to_db() | ||
def _get_relaited_obj_field_name(self, shape_type): | ||
'polygon_paths', 'polyline_paths', 'points_paths', 'box_paths' |
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.
Is it extra line?
@@ -51,6 +57,8 @@ class ShapeCollectionModel extends Listener { | |||
this._colorIdx = 0; | |||
this._filter = new FilterModel(() => this.update()); | |||
this._splitter = new ShapeSplitter(); | |||
this._erased = false; | |||
this._initialShapes = []; |
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.
This field is list, but in this.updateHash() the same name is used as dictionary
@@ -221,49 +229,84 @@ class ShapeCollectionModel extends Listener { | |||
return this; | |||
} | |||
|
|||
_getExportTargetContainer(export_type, shape_type, container) { |
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.
Are you sure it is should be a method? It don't uses any "this" context.
@@ -801,6 +878,10 @@ class ShapeCollectionModel extends Listener { | |||
get shapes() { | |||
return this._shapes; | |||
} | |||
|
|||
get erased() { |
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.
unusable getter
for (const shape of this._shapes) { | ||
let target_export_container = undefined; | ||
if (!shape._removed) { | ||
if (!(shape.id in this._initialShapes)) { |
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 (!(shape.id in this._initialShapes) || this._erased) {
continue; | ||
} | ||
} | ||
else if (shape.id in this._initialShapes) { |
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.
(shape.id in this._initialShapes && !this._erased)
@@ -77,7 +79,8 @@ class AnnotationParser { | |||
|
|||
let occluded = +shape.getAttribute('occluded'); | |||
let z_order = shape.getAttribute('z_order') || '0'; | |||
return [points, occluded, +z_order]; | |||
let client_id = box.getAttribute('client_id') || '-1' |
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.
Need to check somehow that client_id is valid. For example, we don't have the same client id for different boxes or all of them are -1. The check should be on client side when we parse annotation and on server side when the user presses down key.
|
||
const data = { | ||
annotation: exportedData, | ||
annotation: JSON.stringify(exportedData), |
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.
What is the reason to send it as string?
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.
I think cause of issue was hash function, that used early. I've not changed request format for backward capability and suggest to change it in new MR.
shape_container_target = export_action_container.polyline_paths; | ||
} | ||
|
||
if (!shape_container_target) { |
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.
Does switch has 'default'? Does it make sense to move the statement into default?
this._erased = false; | ||
} | ||
|
||
export() { |
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.
How long does the function works on big tasks like FP/FN?
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.
About 600ms for 90k objects
cvat/apps/engine/annotation.py
Outdated
client_ids = set() | ||
def extract_and_check_clinet_id(obj): | ||
if 'client_id' not in box: | ||
raise Exception('No client_id field in received data') |
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.
Please fix indentation
cvat/apps/engine/annotation.py
Outdated
if 'client_id' not in box: | ||
raise Exception('No client_id field in received data') | ||
client_id = obj['client_id'] | ||
if obj['client_id'] in client_ids: |
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.
client_id in client_ids
cvat/apps/engine/annotation.py
Outdated
client_ids.add(client_id) | ||
return client_id | ||
|
||
for action in ['create', 'update', 'delete']: |
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.
Let's roll the loop
cvat/apps/engine/annotation.py
Outdated
@@ -1707,7 +2065,8 @@ def _flip_shape(shape, im_w, im_h): | |||
("ytl", "{:.2f}".format(shape.ytl)), | |||
("xbr", "{:.2f}".format(shape.xbr)), | |||
("ybr", "{:.2f}".format(shape.ybr)), | |||
("occluded", str(int(shape.occluded))) | |||
("occluded", str(int(shape.occluded))), | |||
("client_id", "{}".format(shape.client_id)), |
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.
str(shape.client_id)
cvat/apps/engine/annotation.py
Outdated
@@ -1726,7 +2085,8 @@ def _flip_shape(shape, im_w, im_h): | |||
"{:.2f}".format(float(p.split(',')[1])) | |||
)) for p in shape.points.split(' ')) | |||
)), | |||
("occluded", str(int(shape.occluded))) | |||
("occluded", str(int(shape.occluded))), | |||
("client_id", "{}".format(shape.client_id)), |
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.
Please use str
cvat/apps/engine/annotation.py
Outdated
@@ -1773,7 +2133,8 @@ def _flip_shape(shape, im_w, im_h): | |||
for path in path_list: | |||
dump_dict = OrderedDict([ | |||
("id", str(path_idx)), | |||
("label", path.label.name) | |||
("label", path.label.name), | |||
("client_id", "{}".format(path.client_id)), |
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.
Please use str
@azhavoro
|
@azhavoro |
@azhavoro
|
@azhavoro |
* Bug has been fixed: impossible to lock/occlude object in AAM * Bug has been fixed: invisible points actually are visible * Bug has been fixed: impossible to close points after editing (#98) * doc: grammatical cleanup of README.md (#107) * Add info about development environment into CONTRIBUTING.md (#110) * Now we store virtual URL instead of update it in the browser address bar (#112) * Copy URL, Frame URL and object URL functionality in a context menu * Bug has been fixed: label UIs don't update after changelabel (#109) * Common escape button for exit from creating/groupping/merging/pasting/aam * Switch outside/keyframe shortkeys * Fix django vulnerability (#121) * Add analytics component (#118) * Incremental save of annotations (#120) * Create task timeout 1h -> 4h. (#136) * OpenVino integration (#134) * Update README.md (#138) * Add an extra field into meta section of a dump file (#149) * Job status was implemented (#153) * Back link to task from annotation view (#156) * Change a task with labels and attributes in admin panel (#157) * Permissions per tasks and jobs (#185) * Fix context menu, text visibility for small images (#202) * Fixed: both context menu are opened simultaneously * Fixed: shape can be unavailable behind text * Fixed: invisible text outside frame * Fix upload big xml files for tasks (#199) * Add Questions section to Readme.md (#226) * Fixed labels order (#242) * Propagate behaviour has been updated in cases with a different resolution (#246) * Updated the guide and images (#241) * Fix number attribute for float numbers. (#258)
No description provided.