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

Task moving between projects #3164

Merged
merged 39 commits into from
May 20, 2021
Merged

Task moving between projects #3164

merged 39 commits into from
May 20, 2021

Conversation

ActiveChooN
Copy link
Contributor

@ActiveChooN ActiveChooN commented May 6, 2021

Motivation and context

Added task moving between projects.
Resolve #2972.

The moving process supports moving a task from outside into a project with mapping task and project and from project to project. Attributes are clearing now during the moving process. In case of label mismatch, you can create or delete necessary labels in the project/task. Many labels can be mapped to one in the target project.

изображение

How has this been tested?

Manually.

Checklist

License

  • I submit my code changes under the same MIT License that covers the project.
    Feel free to contact the maintainers if that's a concern.
  • I have updated the license header for each file (see an example below)
# Copyright (C) 2021 Intel Corporation
#
# SPDX-License-Identifier: MIT

@ActiveChooN ActiveChooN added enhancement New feature or request need test labels May 11, 2021
@ActiveChooN ActiveChooN changed the title [WIP] Task moving between projects Task moving between projects May 12, 2021
@ActiveChooN ActiveChooN requested a review from azhavoro May 12, 2021 16:15
bsekachev
bsekachev previously approved these changes May 18, 2021
Copy link
Member

@bsekachev bsekachev left a comment

Choose a reason for hiding this comment

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

Client part LGTM

@bsekachev bsekachev self-requested a review May 18, 2021 09:55
@nmanovic
Copy link
Contributor

nmanovic commented May 20, 2021

@ActiveChooN , could you please resolve issues with Remark and ESLinter? Why did we get them? If it is necessary, please consult with @dvkruchinin .

@nmanovic
Copy link
Contributor

nmanovic commented May 20, 2021

@dvkruchinin , tests were failed after develop merge. Could you please take a look?

cvat-core/package.json Outdated Show resolved Hide resolved
@nmanovic
Copy link
Contributor

@ActiveChooN , could you remind me why clear attributes are always on?

@dvkruchinin
Copy link
Contributor

tests were failed after develop merge. Could you please take a look?

In the corresponding workflow run, there were problems with the cvat-ui container build. It is worth waiting for the completion of the current launch.

if instance.project_id is None:
for old_label in instance.label_set.all():
try:
new_label = project.label_set.filter(name=old_label.name).first()
Copy link
Contributor

Choose a reason for hiding this comment

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

For every label it is a separate DB call. Probably it isn't a problem for now. Let's keep it as is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In that case, there are already 4 DB calls for every label at least. I will try to improve it.

@ActiveChooN
Copy link
Contributor Author

@ActiveChooN, could you remind me why clear attributes are always on?

It's a boilerplate for future feature development. Now only a simple moving algorithm is available witch will wipe label attributes while moving. But in the next PRs, it can include attributes mapping too. And it's a reminder for users that their labels will be deleted.

new_label = project.label_set.filter(name=old_label.name).first()
except ValueError:
raise serializers.ValidationError(f'Target project does not have label with name "{old_label.name}"')
old_label.attributespec_set.all().delete()
Copy link
Contributor

Choose a reason for hiding this comment

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

In some specific cases we want to transfer attributes as well. For example, when we have identical definition for labels and attributes. Are you going to support that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but it's quite an independent feature so in future implementations

@ActiveChooN
Copy link
Contributor Author

@ActiveChooN, could you please resolve issues with Remark and ESLinter? Why did we get them? If it is necessary, please consult with @dvkruchinin.

Fixed for ESLint but have no idea, why remark getting issues with CHANGELOG.md with the template for the 1.5.0 version.

@bsekachev
Copy link
Member

Fixed for ESLint but have no idea, why remark getting issues with CHANGELOG.md with the template for the 1.5.0 version.

@ActiveChooN
Need to delete empty markers -. Otherwise remark warns about them.

@nmanovic nmanovic merged commit a2df499 into develop May 20, 2021
@nmanovic nmanovic deleted the dk/tasks-move-between-projects branch May 20, 2021 09:44
@ActiveChooN
Copy link
Contributor Author

@TOsmanov, @aschernov, could you add documentation to cover this feature to the "Creating an annotation task" page, please?

@aschernov
Copy link
Contributor

@ActiveChooN , yes, we'll take it in work.

@ActiveChooN
Copy link
Contributor Author

@aschernov, hi, have you any updates or time estimation for this feature?

@aschernov
Copy link
Contributor

aschernov commented May 28, 2021

@ActiveChooN , Hi, it was implemented here #3232 , or what do you mean?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

After update to v1.2.0 tasks created prior to update cannot be used with projects
6 participants