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

Manifest fixes #3146

Merged
merged 11 commits into from
May 4, 2021
Merged

Manifest fixes #3146

merged 11 commits into from
May 4, 2021

Conversation

Marishka17
Copy link
Contributor

@Marishka17 Marishka17 commented Apr 29, 2021

Motivation and context

Resolves #3119

  • Fixed missed force flag for preparing video meta
  • Fixed image filtering
  • Added reverse function for migration

How has this been tested?

In development environment, docker

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

@Marishka17 Marishka17 changed the title [WIP] Manifest fixes Manifest fixes Apr 30, 2021
@nmanovic
Copy link
Contributor

@Marishka17 , could you please add a line into CHANGELOG.md?

azhavoro
azhavoro previously approved these changes May 4, 2021
from utils.dataset_manifest import ImageManifestManager, VideoManifestManager

def migrate_data(apps, shema_editor):
def get_logger():
migration = os.path.basename(__file__).split(".")[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

Does splitext work here? But I'm fine with the solution as well.

def get_logger():
migration = os.path.basename(__file__).split(".")[0]
logger = logging.getLogger(name=migration)
logger.setLevel(logging.INFO)
Copy link
Contributor

Choose a reason for hiding this comment

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

We have a special file with log configs. Do you think we can put the code into https://github.com/openvinotoolkit/cvat/blob/develop/cvat/apps/engine/log.py?

@nmanovic
Copy link
Contributor

nmanovic commented May 4, 2021

@Marishka17 , I believe the PR is good. It is a good idea to use logging inside some migrations. Better to move the initialization code into a common place. Please fix and after that I'm OK with the PR. Great job!

@nmanovic
Copy link
Contributor

nmanovic commented May 4, 2021

I will merge the PR now to unblock release of CVAT 1.4.0. Please prepare a separate PR with improved logging.

@nmanovic nmanovic merged commit e7cca0e into develop May 4, 2021
@nmanovic nmanovic deleted the mk/manifest_fix branch May 4, 2021 16:53
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.

Could not fetch frame data from the server (A manifest file not exists)
3 participants