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

Add tf import checks and tests #1567

Merged
merged 12 commits into from
Jun 4, 2020
22 changes: 22 additions & 0 deletions cvat-core/src/annotation-formats.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
name: initialData.name,
format: initialData.ext,
version: initialData.version,
enabled: initialData.enabled,
};

Object.defineProperties(this, {
Expand Down Expand Up @@ -48,6 +49,16 @@
*/
get: () => data.version,
},
enabled: {
/**
* @name enabled
* @type {string}
* @memberof module:API.cvat.classes.Loader
* @readonly
* @instance
*/
get: () => data.enabled,
},
});
}
}
Expand All @@ -63,6 +74,7 @@
name: initialData.name,
format: initialData.ext,
version: initialData.version,
enabled: initialData.enabled,
};

Object.defineProperties(this, {
Expand Down Expand Up @@ -96,6 +108,16 @@
*/
get: () => data.version,
},
enabled: {
/**
* @name enabled
* @type {string}
* @memberof module:API.cvat.classes.Loader
* @readonly
* @instance
*/
get: () => data.enabled,
},
});
}
}
Expand Down
4 changes: 2 additions & 2 deletions cvat-ui/src/components/actions-menu/actions-menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ interface Props {
taskMode: string;
bugTracker: string;

loaders: string[];
dumpers: string[];
loaders: any[];
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add an interface definition for loader and dumper?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know, can I? I'd just have used the class name, but for some reason they are never used here, so I just followed the style.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bsekachev, can we use more specific types here than any for objects?

Copy link
Member

Choose a reason for hiding this comment

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

@zhiltsov-max

On the one hand, of course, we can declare temporary interfaces in cvat-ui for loaders and dumpers and specify them here.
But on the other hand it is only temporary solution. In general case need to rewrite cvat-core on typescript or at least create typings file for the library. This is quite huge change.

dumpers: any[];
loadActivity: string | null;
dumpActivities: string[] | null;
exportActivities: string[] | null;
Expand Down
15 changes: 8 additions & 7 deletions cvat-ui/src/components/actions-menu/dump-submenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ function isDefaultFormat(dumperName: string, taskMode: string): boolean {
interface Props {
taskMode: string;
menuKey: string;
dumpers: string[];
dumpers: any[];
dumpActivities: string[] | null;
}

Expand All @@ -30,17 +30,18 @@ export default function DumpSubmenu(props: Props): JSX.Element {
return (
<Menu.SubMenu key={menuKey} title='Dump annotations'>
{
dumpers.map((dumper: string): JSX.Element => {
const pending = (dumpActivities || []).includes(dumper);
const isDefault = isDefaultFormat(dumper, taskMode);
dumpers.map((dumper: any): JSX.Element => {
const pending = (dumpActivities || []).includes(dumper.name);
const disabled = !dumper.enabled || pending;
const isDefault = isDefaultFormat(dumper.name, taskMode);
return (
<Menu.Item
key={dumper}
disabled={pending}
key={dumper.name}
disabled={disabled}
className='cvat-menu-dump-submenu-item'
>
<Icon type='download' />
<Text strong={isDefault}>{dumper}</Text>
<Text strong={isDefault}>{dumper.name}</Text>
{pending && <Icon style={{ marginLeft: 10 }} type='loading' />}
</Menu.Item>
);
Expand Down
13 changes: 7 additions & 6 deletions cvat-ui/src/components/actions-menu/export-submenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import Text from 'antd/lib/typography/Text';

interface Props {
menuKey: string;
exporters: string[];
exporters: any[];
exportActivities: string[] | null;
}

Expand All @@ -23,16 +23,17 @@ export default function ExportSubmenu(props: Props): JSX.Element {
return (
<Menu.SubMenu key={menuKey} title='Export as a dataset'>
{
exporters.map((exporter: string): JSX.Element => {
const pending = (exportActivities || []).includes(exporter);
exporters.map((exporter: any): JSX.Element => {
const pending = (exportActivities || []).includes(exporter.name);
const disabled = !exporter.enabled || pending;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bsekachev, can we show disabled formats in gray?

Copy link
Member

Choose a reason for hiding this comment

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

@zhiltsov-max What means "disabled formats"? Maybe just do not show them at all?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Existing, but not available.

return (
<Menu.Item
key={exporter}
disabled={pending}
key={exporter.name}
disabled={disabled}
className='cvat-menu-export-submenu-item'
>
<Icon type='export' />
<Text>{exporter}</Text>
<Text>{exporter.name}</Text>
{pending && <Icon style={{ marginLeft: 10 }} type='loading' />}
</Menu.Item>
);
Expand Down
18 changes: 11 additions & 7 deletions cvat-ui/src/components/actions-menu/load-submenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import Text from 'antd/lib/typography/Text';

interface Props {
menuKey: string;
loaders: string[];
loaders: any[];
loadActivity: string | null;
onFileUpload(file: File): void;
}
Expand All @@ -27,12 +27,16 @@ export default function LoadSubmenu(props: Props): JSX.Element {
return (
<Menu.SubMenu key={menuKey} title='Upload annotations'>
{
loaders.map((_loader: string): JSX.Element => {
const [loader, accept] = _loader.split('::');
const pending = loadActivity === loader;
loaders.map((loader: any): JSX.Element => {
const accept = loader.format
.split(',')
.map((x: string) => '.' + x.trimStart())
.join(', '); // add '.' to each extension in a list
const pending = loadActivity === loader.name;
const disabled = !loader.enabled || pending;
return (
<Menu.Item
key={loader}
key={loader.name}
disabled={!!loadActivity}
className='cvat-menu-load-submenu-item'
>
Expand All @@ -45,9 +49,9 @@ export default function LoadSubmenu(props: Props): JSX.Element {
return false;
}}
>
<Button block type='link' disabled={!!loadActivity}>
<Button block type='link' disabled={!!loadActivity || disabled}>
<Icon type='upload' />
<Text>{loader}</Text>
<Text>{loader.name}</Text>
{pending && <Icon style={{ marginLeft: 10 }} type='loading' />}
</Button>
</Upload>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import ReIDPlugin from './reid-plugin';

interface Props {
taskMode: string;
loaders: string[];
dumpers: string[];
loaders: any[];
dumpers: any[];
loadActivity: string | null;
dumpActivities: string[] | null;
exportActivities: string[] | null;
Expand Down
6 changes: 3 additions & 3 deletions cvat-ui/src/containers/actions-menu/actions-menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ function ActionsMenuContainer(props: OwnProps & StateToProps & DispatchToProps):
dumpAnnotations(taskInstance, dumper);
}
} else if (action === Actions.LOAD_TASK_ANNO) {
const [format] = additionalKey.split('::');
const format = additionalKey;
const [loader] = loaders
.filter((_loader: any): boolean => _loader.name === format);
if (loader && file) {
Expand Down Expand Up @@ -166,8 +166,8 @@ function ActionsMenuContainer(props: OwnProps & StateToProps & DispatchToProps):
taskID={taskInstance.id}
taskMode={taskInstance.mode}
bugTracker={taskInstance.bugTracker}
loaders={loaders.map((loader: any): string => `${loader.name}::${loader.format}`)}
dumpers={dumpers.map((dumper: any): string => dumper.name)}
loaders={loaders}
dumpers={dumpers}
loadActivity={loadActivity}
dumpActivities={dumpActivities}
exportActivities={exportActivities}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ function AnnotationMenuContainer(props: Props): JSX.Element {
dumpAnnotations(jobInstance.task, dumper);
}
} else if (action === Actions.LOAD_JOB_ANNO) {
const [format] = additionalKey.split('::');
const format = additionalKey;
const [loader] = loaders
.filter((_loader: any): boolean => _loader.name === format);
if (loader && file) {
Expand All @@ -150,8 +150,8 @@ function AnnotationMenuContainer(props: Props): JSX.Element {
return (
<AnnotationMenuComponent
taskMode={jobInstance.task.mode}
loaders={loaders.map((loader: any): string => loader.name)}
dumpers={dumpers.map((dumper: any): string => dumper.name)}
loaders={loaders}
dumpers={dumpers}
loadActivity={loadActivity}
dumpActivities={dumpActivities}
exportActivities={exportActivities}
Expand Down
15 changes: 10 additions & 5 deletions cvat/apps/dataset_manager/formats/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ class _Format:
EXT = ''
VERSION = ''
DISPLAY_NAME = '{NAME} {VERSION}'
ENABLED = True

class Exporter(_Format):
def __call__(self, dst_file, task_data, **options):
Expand All @@ -22,7 +23,7 @@ class Importer(_Format):
def __call__(self, src_file, task_data, **options):
raise NotImplementedError()

def _wrap_format(f_or_cls, klass, name, version, ext, display_name):
def _wrap_format(f_or_cls, klass, name, version, ext, display_name, enabled):
import inspect
assert inspect.isclass(f_or_cls) or inspect.isfunction(f_or_cls)
if inspect.isclass(f_or_cls):
Expand All @@ -44,25 +45,29 @@ def __call__(self, *args, **kwargs):
target.DISPLAY_NAME = (display_name or klass.DISPLAY_NAME).format(
NAME=name, VERSION=version, EXT=ext)
assert all([target.NAME, target.VERSION, target.EXT, target.DISPLAY_NAME])
target.ENABLED = enabled

return target

EXPORT_FORMATS = {}
def exporter(name, version, ext, display_name=None):
def exporter(name, version, ext, display_name=None, enabled=True):
assert name not in EXPORT_FORMATS, "Export format '%s' already registered" % name
def wrap_with_params(f_or_cls):
t = _wrap_format(f_or_cls, Exporter,
name=name, ext=ext, version=version, display_name=display_name)
name=name, ext=ext, version=version, display_name=display_name,
enabled=enabled)
key = t.DISPLAY_NAME
assert key not in EXPORT_FORMATS, "Export format '%s' already registered" % name
EXPORT_FORMATS[key] = t
return t
return wrap_with_params

IMPORT_FORMATS = {}
def importer(name, version, ext, display_name=None):
def importer(name, version, ext, display_name=None, enabled=True):
def wrap_with_params(f_or_cls):
t = _wrap_format(f_or_cls, Importer,
name=name, ext=ext, version=version, display_name=display_name)
name=name, ext=ext, version=version, display_name=display_name,
enabled=enabled)
key = t.DISPLAY_NAME
assert key not in IMPORT_FORMATS, "Import format '%s' already registered" % name
IMPORT_FORMATS[key] = t
Expand Down
12 changes: 10 additions & 2 deletions cvat/apps/dataset_manager/formats/tfrecord.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,15 @@
from .registry import dm_env, exporter, importer


@exporter(name='TFRecord', ext='ZIP', version='1.0')
from datumaro.util.tf_util import import_tf
try:
import_tf()
tf_available = True
except ImportError:
tf_available = False


@exporter(name='TFRecord', ext='ZIP', version='1.0', enabled=tf_available)
def _export(dst_file, task_data, save_images=False):
extractor = CvatTaskDataExtractor(task_data, include_images=save_images)
extractor = Dataset.from_extractors(extractor) # apply lazy transforms
Expand All @@ -25,7 +33,7 @@ def _export(dst_file, task_data, save_images=False):

make_zip_archive(temp_dir, dst_file)

@importer(name='TFRecord', ext='ZIP', version='1.0')
@importer(name='TFRecord', ext='ZIP', version='1.0', enabled=tf_available)
def _import(src_file, task_data):
with TemporaryDirectory() as tmp_dir:
Archive(src_file.name).extractall(tmp_dir)
Expand Down
1 change: 1 addition & 0 deletions cvat/apps/dataset_manager/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ class DatasetFormatSerializer(serializers.Serializer):
name = serializers.CharField(max_length=64, source='DISPLAY_NAME')
ext = serializers.CharField(max_length=64, source='EXT')
version = serializers.CharField(max_length=64, source='VERSION')
enabled = serializers.BooleanField(source='ENABLED')

class DatasetFormatsSerializer(serializers.Serializer):
importers = DatasetFormatSerializer(many=True)
Expand Down
6 changes: 6 additions & 0 deletions cvat/apps/dataset_manager/tests/_test_formats.py
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,9 @@ def check(file_path):
self.assertTrue(len(f.read()) != 0)

for f in dm.views.get_export_formats():
if not f.ENABLED:
self.skipTest("Format is disabled")

format_name = f.DISPLAY_NAME
for save_images in { True, False }:
with self.subTest(format=format_name, save_images=save_images):
Expand All @@ -359,6 +362,9 @@ def test_empty_images_are_exported(self):
('YOLO 1.1', 'yolo'),
]:
with self.subTest(format=format_name):
if not dm.formats.registry.EXPORT_FORMATS[format_name].ENABLED:
self.skipTest("Format is disabled")

task = self._generate_task()

def check(file_path):
Expand Down
17 changes: 11 additions & 6 deletions cvat/apps/engine/tests/_test_rest_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -3149,8 +3149,8 @@ def _get_initial_annotation(annotation_format):
export_formats = data['exporters']
self.assertTrue(isinstance(import_formats, list) and import_formats)
self.assertTrue(isinstance(export_formats, list) and export_formats)
import_formats = { v['name'] for v in import_formats }
export_formats = { v['name'] for v in export_formats }
import_formats = { v['name']: v for v in import_formats }
export_formats = { v['name']: v for v in export_formats }

formats = { exp: exp if exp in import_formats else None
for exp in export_formats }
Expand All @@ -3159,12 +3159,12 @@ def _get_initial_annotation(annotation_format):
formats['CVAT for video 1.1'] = 'CVAT 1.1'
if 'CVAT for images 1.1' in export_formats:
formats['CVAT for images 1.1'] = 'CVAT 1.1'
if import_formats ^ export_formats:
if set(import_formats) ^ set(export_formats):
# NOTE: this may not be an error, so we should not fail
print("The following import formats have no pair:",
import_formats - export_formats)
set(import_formats) - set(export_formats))
print("The following export formats have no pair:",
export_formats - import_formats)
set(export_formats) - set(import_formats))

for export_format, import_format in formats.items():
with self.subTest(export_format=export_format,
Expand All @@ -3183,7 +3183,12 @@ def _get_initial_annotation(annotation_format):
# 3. download annotation
response = self._dump_api_v1_tasks_id_annotations(task["id"], annotator,
"?format={}".format(export_format))
self.assertEqual(response.status_code, HTTP_202_ACCEPTED)
if annotator and not export_formats[export_format]['enabled']:
self.assertEqual(response.status_code,
status.HTTP_405_METHOD_NOT_ALLOWED)
continue
else:
self.assertEqual(response.status_code, HTTP_202_ACCEPTED)

response = self._dump_api_v1_tasks_id_annotations(task["id"], annotator,
"?format={}".format(export_format))
Expand Down