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
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added
- Throttling policy for unauthenticated users (<https://github.com/opencv/cvat/pull/1531>)
- Added default label color table for mask export (https://github.com/opencv/cvat/pull/1549)
- Added visual identification for unavailable formats (https://github.com/opencv/cvat/pull/1567)

### Changed
- Removed information about e-mail from the basic user information (<https://github.com/opencv/cvat/pull/1627>)
Expand All @@ -19,7 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
-

### Fixed
-
- Fixed interpreter crash when trying to import `tensorflow` with no AVX instructions available (https://github.com/opencv/cvat/pull/1567)

### Security
-
Expand Down
2 changes: 1 addition & 1 deletion cvat-core/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "cvat-core",
"version": "2.0.1",
"version": "2.1.1",
"description": "Part of Computer Vision Tool which presents an interface for client-side integration",
"main": "babel.config.js",
"scripts": {
Expand Down
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
2 changes: 1 addition & 1 deletion cvat-ui/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion cvat-ui/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "cvat-ui",
"version": "1.2.0",
"version": "1.2.1",
"description": "CVAT single-page application",
"main": "src/index.tsx",
"scripts": {
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
18 changes: 11 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,21 @@ 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
.sort((a: any, b: any) => a.name.localeCompare(b.name))
.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} disabled={disabled}>{dumper.name}</Text>
Copy link
Member

Choose a reason for hiding this comment

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

You can try something like that: type={disabled ? 'secondary' : undefined} if you want to do text gray.
But I would suggest do not display such formats. Just using:

dumpers.filter((dumper: any) => dumper.enabled).sort(...

Copy link
Member

Choose a reason for hiding this comment

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

One more option is to use style attribute:

<Text style={disabled ? { opacity: '0.5' } : {}} ...

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 tried to just pass disabled to Icon element, but it didn't work, while it is described in the documentation of the library. I guess, we can be using an outdated version.

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

We use version 3.x. It was the latest version when we started doing this UI.
Now version 4.x exists, but migrating is painful

{pending && <Icon style={{ marginLeft: 10 }} type='loading' />}
</Menu.Item>
);
Expand Down
16 changes: 10 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,20 @@ 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
.sort((a: any, b: any) => a.name.localeCompare(b.name))
.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 disabled={disabled}>{exporter.name}</Text>
{pending && <Icon style={{ marginLeft: 10 }} type='loading' />}
</Menu.Item>
);
Expand Down
23 changes: 15 additions & 8 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,13 +27,20 @@ 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
.sort((a: any, b: any) => a.name.localeCompare(b.name))
.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 || !!loadActivity;
return (
<Menu.Item
key={loader}
disabled={!!loadActivity}
key={loader.name}
disabled={disabled}
className='cvat-menu-load-submenu-item'
>
<Upload
Expand All @@ -45,9 +52,9 @@ export default function LoadSubmenu(props: Props): JSX.Element {
return false;
}}
>
<Button block type='link' disabled={!!loadActivity}>
<Button block type='link' disabled={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