Add tf import checks and tests #1567
Conversation
Pull Request Test Coverage Report for Build 5468
💛 - Coveralls |
| const pending = (exportActivities || []).includes(exporter); | ||
| exporters.map((exporter: any): JSX.Element => { | ||
| const pending = (exportActivities || []).includes(exporter.name); | ||
| const disabled = !exporter.enabled || pending; |
There was a problem hiding this comment.
@bsekachev, can we show disabled formats in gray?
There was a problem hiding this comment.
@zhiltsov-max What means "disabled formats"? Maybe just do not show them at all?
There was a problem hiding this comment.
Existing, but not available.
zhiltsov-max
changed the title
|
@bsekachev, can you review this patch? |
|
@zhiltsov-max , I see a couple of conflicts. |
| @@ -16,8 +16,8 @@ interface Props { | |||
| taskMode: string; | |||
| bugTracker: string; | |||
|
|
|||
| loaders: string[]; | |||
| dumpers: string[]; | |||
| loaders: any[]; | |||
There was a problem hiding this comment.
Could you add an interface definition for loader and dumper?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@bsekachev, can we use more specific types here than any for objects?
There was a problem hiding this comment.
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.
| className='cvat-menu-dump-submenu-item' | ||
| > | ||
| <Icon type='download' /> | ||
| <Text strong={isDefault}>{dumper}</Text> | ||
| <Text strong={isDefault} disabled={disabled}>{dumper.name}</Text> |
There was a problem hiding this comment.
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(...
There was a problem hiding this comment.
One more option is to use style attribute:
<Text style={disabled ? { opacity: '0.5' } : {}} ...
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
|
@bsekachev, they actually are. The icons in exports are not, but the text is. |
|
Can I merge it? |
|
@bsekachev, it is ready. |
* Add tf import checks and tests * implement disabled formats on server * python 3.5 compatibility * add checks to dm tests * fix tests * Support for disabled formats in UI * add sorting for formats, mark grey disabled items * update changelog * advance package versions
Closes #1393
Closes #1219
Closes #970
Motivation and context
When
tensorflowpackage was imported, it was causing crash of Python interpreter if there was no AVX support on the machine. This patch adds a check for availability of such import. If a format can not be used, it is shown as inactive.How has this been tested?
Unit tests.
Checklist
developbranchcvat-core, cvat-data and cvat-ui)
License
Feel free to contact the maintainers if that's a concern.