From 49872f69920aab300f55cede2ef7bffd72506317 Mon Sep 17 00:00:00 2001 From: Brandon Davis Date: Mon, 28 Feb 2022 19:10:25 -0500 Subject: [PATCH 1/8] Fix image map loading logic on desktop --- .../desktop/backend/serializers/viame.spec.ts | 22 +++++++++-- .../desktop/backend/serializers/viame.ts | 39 +++++++++++++------ 2 files changed, 46 insertions(+), 15 deletions(-) diff --git a/client/platform/desktop/backend/serializers/viame.spec.ts b/client/platform/desktop/backend/serializers/viame.spec.ts index 419a16b93..5ae07214c 100644 --- a/client/platform/desktop/backend/serializers/viame.spec.ts +++ b/client/platform/desktop/backend/serializers/viame.spec.ts @@ -15,7 +15,7 @@ const testData: testPairs[] = fs.readJSONSync('../testutils/viame.spec.json'); const imageFilenameTests = [ { pass: false, - error: 'There was a mixture of fields that specified image names and fields that did not. Please check the CSV', + error: 'CSV import was found to have a mix of missing images and images that were found in the data. This usually indicates a problem with the annotation file, but if you want to force the import to proceed, you can set all values in the Image Name column to be blank. Then DIVE will not attempt to validate image names. Missing images include: ...', csv: [ '0, ,1,884.66,510,1219.66,737.66,1,-1,ignored,0.98', '1,2.png,0,111,222,3333,444,1,-1,typestring,0.55', @@ -23,10 +23,18 @@ const imageFilenameTests = [ }, { pass: false, - error: 'There was a mixture of fields that specified image names and fields that did not. Please check the CSV', + error: 'CSV import was found to have a mix of missing images and images that were found in the data. This usually indicates a problem with the annotation file, but if you want to force the import to proceed, you can set all values in the Image Name column to be blank. Then DIVE will not attempt to validate image names. Missing images include: invalid...', csv: [ '0,1.png,1,884.66,510,1219.66,737.66,1,-1,ignored,0.98', - '1,,0,111,222,3333,444,1,-1,typestring,0.55', + '1,invalid,0,111,222,3333,444,1,-1,typestring,0.55', + ], + }, + { + pass: true, + csv: [ + '0,invalid1,1,884.66,510,1219.66,737.66,1,-1,ignored,0.98', + '', + '1,invalid2,0,111,222,3333,444,1,-1,typestring,0.55', ], }, { @@ -37,6 +45,14 @@ const imageFilenameTests = [ '1,,0,111,222,3333,444,1,-1,typestring,0.55', ], }, + { + pass: true, + csv: [ + '0,1.png,1,884.66,510,1219.66,737.66,1,-1,ignored,0.98', + '', + '1,1.png,0,111,222,3333,444,1,-1,typestring,0.55', + ], + }, ]; diff --git a/client/platform/desktop/backend/serializers/viame.ts b/client/platform/desktop/backend/serializers/viame.ts index 9239ad13c..f76646f08 100644 --- a/client/platform/desktop/backend/serializers/viame.ts +++ b/client/platform/desktop/backend/serializers/viame.ts @@ -246,6 +246,7 @@ async function parse(input: Readable, imageMap?: Map): Promise(); + const missingImages: string[] = []; let reordered = false; return new Promise((resolve, reject) => { @@ -254,31 +255,45 @@ async function parse(input: Readable, imageMap?: Map): Promise 0 && missingImages.length !== tracks.length) { + /** + * If missing image count was different than track length, then some number of images + * from column 2 were actually valid and some were not. This indicates that the dataset + * being loaded is probably corrupt. + * + * If their counts match perfectly, then every single image was missing, which indicates + * that the dataset either had all empty values in column 2 or some other type of invalid + * string that should not prevent import. + */ + reject([ + 'CSV import was found to have a mix of missing images and images that were found', + 'in the data. This usually indicates a problem with the annotation file, but if', + 'you want to force the import to proceed, you can set all values in the', + 'Image Name column to be blank. Then DIVE will not attempt to validate image names.', + `Missing images include: ${missingImages.slice(0, 5)}...`, + ].join(' ')); + } + } + resolve({ tracks, fps }); }); parser.on('readable', () => { let record: string[]; - let hasFilenames: undefined | boolean; // eslint-disable-next-line no-cond-assign while (record = parser.read()) { try { const { rowInfo, feature, trackAttributes, confidencePairs, } = _parseFeature(record); - const currentHasFileName = rowInfo.filename.trim() !== ''; - if (imageMap !== undefined && hasFilenames === undefined) { - hasFilenames = currentHasFileName; - } else if (imageMap !== undefined && hasFilenames !== currentHasFileName) { - throw new Error('Image Filenames specified in the Column 2 of the CSV must either be all set or all empty. Encountered a mixture of set and empty filenames'); - } - if (imageMap !== undefined && hasFilenames) { + if (imageMap !== undefined) { // validate image ordering if the imageMap is provided and a non-whitespace filename const [imageName] = splitExt(rowInfo.filename); const expectedFrameNumber = imageMap.get(imageName); if (expectedFrameNumber === undefined) { - throw new Error( - `encountered annotation for image not found in dataset: ${rowInfo.filename}`, - ); + missingImages.push(rowInfo.filename); + // console.log(record); } else if (expectedFrameNumber !== feature.frame) { // force reorder the annotations reordered = true; From e9d97cd1668071cf7bca9e747688ee7862bee44b Mon Sep 17 00:00:00 2001 From: Brandon Davis Date: Mon, 28 Feb 2022 19:11:59 -0500 Subject: [PATCH 2/8] Remove comment --- client/platform/desktop/backend/serializers/viame.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/client/platform/desktop/backend/serializers/viame.ts b/client/platform/desktop/backend/serializers/viame.ts index f76646f08..7570fe27a 100644 --- a/client/platform/desktop/backend/serializers/viame.ts +++ b/client/platform/desktop/backend/serializers/viame.ts @@ -293,7 +293,6 @@ async function parse(input: Readable, imageMap?: Map): Promise Date: Mon, 28 Feb 2022 21:31:11 -0500 Subject: [PATCH 3/8] Do the python part --- server/dive_utils/serializers/viame.py | 42 +++++++++++++----------- server/tests/test_serialize_viame_csv.py | 22 +++++++++++-- 2 files changed, 41 insertions(+), 23 deletions(-) diff --git a/server/dive_utils/serializers/viame.py b/server/dive_utils/serializers/viame.py index 0990c4fc6..f4f3bd5e5 100644 --- a/server/dive_utils/serializers/viame.py +++ b/server/dive_utils/serializers/viame.py @@ -248,7 +248,7 @@ def load_csv_as_tracks_and_attributes( metadata_attributes: Dict[str, Dict[str, Any]] = {} test_vals: Dict[str, Dict[str, int]] = {} reordered = False - has_image_filenames: Union[None, bool] = None + missingImages: List[str] = [] for row in reader: if len(row) == 0 or row[0].startswith('#'): # This is not a data row @@ -261,23 +261,13 @@ def load_csv_as_tracks_and_attributes( ) = _parse_row_for_tracks(row) trackId, imageFile, _, _, _ = row_info(row) - current_has_filename = imageFile.strip() != '' - if has_image_filenames is None and imageMap: - has_image_filenames = current_has_filename - elif imageMap and has_image_filenames != current_has_filename: - raise ValueError( - 'There was a mixture of fields that specified image names and fields that' - ' did not. Please check the CSV' - ) - return - if imageMap and has_image_filenames: + + if imageMap: # validate image ordering if the imageMap is provided imageName, _ = os.path.splitext(os.path.basename(imageFile)) expectedFrameNumber = imageMap.get(imageName) if expectedFrameNumber is None: - raise ValueError( - f'encountered annotation for image not found in dataset: {imageFile}' - ) + missingImages.append(imageFile) elif expectedFrameNumber is not feature.frame: # force reorder the annotations reordered = True @@ -305,10 +295,27 @@ def load_csv_as_tracks_and_attributes( create_attributes(metadata_attributes, test_vals, 'track', key, val) for (key, val) in attributes.items(): create_attributes(metadata_attributes, test_vals, 'detection', key, val) + + trackarr = tracks.items() + + if imageMap and len(missingImages) and len(missingImages) != len(trackarr): + examples = ', '.join(missingImages[:3]) + raise ValueError( + ' '.join( + [ + 'CSV import was found to have a mix of missing images and images that', + 'were found in the data. This usually indicates a problem with the', + 'annotation file, but if you want to force the import to proceed, you', + 'can set all values in the Image Name column to be blank. Then DIVE', + 'will not attempt to validate image names.', + f'Missing images include: {examples}...', + ] + ) + ) # Now we process all the metadata_attributes for the types calculate_attribute_types(metadata_attributes, test_vals) - track_json = {trackId: track.dict(exclude_none=True) for trackId, track in tracks.items()} + track_json = {trackId: track.dict(exclude_none=True) for trackId, track in trackarr} return track_json, metadata_attributes @@ -325,15 +332,10 @@ def export_tracks_as_csv( Export track json to a CSV format. :param excludeBelowThreshold: omit tracks below a certain confidence. Requires thresholds. - :param thresholds: key/value pairs with threshold values - :param filenames: list of string file names. filenames[n] should be the image at frame n - :param fps: if FPS is set, column 2 will be video timestamp derived from (frame / fps) - :param header: include or omit header - :param typeFilter: set of track types to only export if not empty """ if thresholds is None: diff --git a/server/tests/test_serialize_viame_csv.py b/server/tests/test_serialize_viame_csv.py index 2902d602f..355b00d74 100644 --- a/server/tests/test_serialize_viame_csv.py +++ b/server/tests/test_serialize_viame_csv.py @@ -278,7 +278,7 @@ image_filename_tests = [ { 'pass': False, - 'error': 'There was a mixture of fields that specified image names and fields that did not. Please check the CSV', + 'error': 'CSV import was found to have a mix of missing images and images that were found in the data. This usually indicates a problem with the annotation file, but if you want to force the import to proceed, you can set all values in the Image Name column to be blank. Then DIVE will not attempt to validate image names. Missing images include: ...', 'csv': [ '0, ,1,884.66,510,1219.66,737.66,1,-1,ignored,0.98', '1,2.png,0,111,222,3333,444,1,-1,typestring,0.55', @@ -286,10 +286,18 @@ }, { 'pass': False, - 'error': 'There was a mixture of fields that specified image names and fields that did not. Please check the CSV', + 'error': 'CSV import was found to have a mix of missing images and images that were found in the data. This usually indicates a problem with the annotation file, but if you want to force the import to proceed, you can set all values in the Image Name column to be blank. Then DIVE will not attempt to validate image names. Missing images include: invalid...', 'csv': [ '0,1.png,1,884.66,510,1219.66,737.66,1,-1,ignored,0.98', - '1,,0,111,222,3333,444,1,-1,typestring,0.55', + '1,invalid,0,111,222,3333,444,1,-1,typestring,0.55', + ], + }, + { + 'pass': True, + 'csv': [ + '0,invalid1,1,884.66,510,1219.66,737.66,1,-1,ignored,0.98', + '', + '1,invalid2,0,111,222,3333,444,1,-1,typestring,0.55', ], }, { @@ -300,6 +308,14 @@ '1,,0,111,222,3333,444,1,-1,typestring,0.55', ], }, + { + 'pass': True, + 'csv': [ + '0,1.png,1,884.66,510,1219.66,737.66,1,-1,ignored,0.98', + '', + '1,1.png,0,111,222,3333,444,1,-1,typestring,0.55', + ], + }, ] From 4281f6e482c2fe7ae6bc1f168cc0bd6bc452d332 Mon Sep 17 00:00:00 2001 From: Brandon Davis Date: Tue, 1 Mar 2022 11:40:25 -0500 Subject: [PATCH 4/8] Check in failing tests --- .../desktop/backend/serializers/viame.spec.ts | 16 ++++++++++++++++ server/tests/test_serialize_viame_csv.py | 15 +++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/client/platform/desktop/backend/serializers/viame.spec.ts b/client/platform/desktop/backend/serializers/viame.spec.ts index 5ae07214c..7da5a7c2d 100644 --- a/client/platform/desktop/backend/serializers/viame.spec.ts +++ b/client/platform/desktop/backend/serializers/viame.spec.ts @@ -53,6 +53,22 @@ const imageFilenameTests = [ '1,1.png,0,111,222,3333,444,1,-1,typestring,0.55', ], }, + { + pass: false, + error: 'images were provided in an unexpected order and dataset contains multi-frame tracks.', + csv: [ + '99,1.png,0,884.66,510,1219.66,737.66,1,-1,ignored,0.98', + '99,3.png,1,111,222,3333,444,1,-1,typestring,0.55', + ], + }, + { + + pass: false, + csv: [ + '99,unknown1,2,884.66,510,1219.66,737.66,1,-1,ignored,0.98', + '99,unknown2,2,111,222,3333,444,1,-1,typestring,0.55', + ], + }, ]; diff --git a/server/tests/test_serialize_viame_csv.py b/server/tests/test_serialize_viame_csv.py index 355b00d74..d3663aa53 100644 --- a/server/tests/test_serialize_viame_csv.py +++ b/server/tests/test_serialize_viame_csv.py @@ -316,6 +316,21 @@ '1,1.png,0,111,222,3333,444,1,-1,typestring,0.55', ], }, + { + 'pass': False, + 'error': 'images were provided in an unexpected order and dataset contains multi-frame tracks.', + 'csv': [ + '99,1.png,0,884.66,510,1219.66,737.66,1,-1,ignored,0.98', + '99,3.png,1,111,222,3333,444,1,-1,typestring,0.55', + ], + }, + { + 'pass': True, + 'csv': [ + '99,unknown1,2,884.66,510,1219.66,737.66,1,-1,ignored,0.98', + '99,unknown2,2,111,222,3333,444,1,-1,typestring,0.55', + ], + }, ] From 1861e3b0ba1e553c892e82c7c935e22c348a1b88 Mon Sep 17 00:00:00 2001 From: Brandon Davis Date: Tue, 1 Mar 2022 13:05:33 -0500 Subject: [PATCH 5/8] Fix tests --- .../desktop/backend/serializers/viame.spec.ts | 7 ++- .../desktop/backend/serializers/viame.ts | 47 +++++++++---------- server/dive_utils/serializers/viame.py | 6 ++- 3 files changed, 31 insertions(+), 29 deletions(-) diff --git a/client/platform/desktop/backend/serializers/viame.spec.ts b/client/platform/desktop/backend/serializers/viame.spec.ts index 7da5a7c2d..8f7dc10cf 100644 --- a/client/platform/desktop/backend/serializers/viame.spec.ts +++ b/client/platform/desktop/backend/serializers/viame.spec.ts @@ -55,15 +55,14 @@ const imageFilenameTests = [ }, { pass: false, - error: 'images were provided in an unexpected order and dataset contains multi-frame tracks.', + error: 'Error: annotations were provided in an unexpected order and dataset contains multi-frame tracks', csv: [ '99,1.png,0,884.66,510,1219.66,737.66,1,-1,ignored,0.98', '99,3.png,1,111,222,3333,444,1,-1,typestring,0.55', ], }, { - - pass: false, + pass: true, csv: [ '99,unknown1,2,884.66,510,1219.66,737.66,1,-1,ignored,0.98', '99,unknown2,2,111,222,3333,444,1,-1,typestring,0.55', @@ -305,7 +304,7 @@ describe('Test Image Filenames', () => { // eslint-disable-next-line no-await-in-loop await parseFile(testPath, imageMap); } catch (err) { - expect(err).toBe(imageOrderData.error); + expect((err as Error).toString()).toBe(imageOrderData.error); } } else { // eslint-disable-next-line no-await-in-loop diff --git a/client/platform/desktop/backend/serializers/viame.ts b/client/platform/desktop/backend/serializers/viame.ts index 7570fe27a..ff122294a 100644 --- a/client/platform/desktop/backend/serializers/viame.ts +++ b/client/platform/desktop/backend/serializers/viame.ts @@ -248,34 +248,33 @@ async function parse(input: Readable, imageMap?: Map): Promise(); const missingImages: string[] = []; let reordered = false; + let anyImageMatched = false; return new Promise((resolve, reject) => { - pipeline(input, parser, (err) => { + pipeline([input, parser], (err) => { // undefined err indicates successful exit if (err !== undefined) { reject(err); } const tracks = Array.from(dataMap.values()); - if (imageMap !== undefined) { - if (missingImages.length > 0 && missingImages.length !== tracks.length) { - /** - * If missing image count was different than track length, then some number of images - * from column 2 were actually valid and some were not. This indicates that the dataset - * being loaded is probably corrupt. - * - * If their counts match perfectly, then every single image was missing, which indicates - * that the dataset either had all empty values in column 2 or some other type of invalid - * string that should not prevent import. - */ - reject([ - 'CSV import was found to have a mix of missing images and images that were found', - 'in the data. This usually indicates a problem with the annotation file, but if', - 'you want to force the import to proceed, you can set all values in the', - 'Image Name column to be blank. Then DIVE will not attempt to validate image names.', - `Missing images include: ${missingImages.slice(0, 5)}...`, - ].join(' ')); - } + if (imageMap !== undefined && missingImages.length > 0 && anyImageMatched) { + /** + * If missing image count was different than track length, then some number of images + * from column 2 were actually valid and some were not. This indicates that the dataset + * being loaded is probably corrupt. + * + * If their counts match perfectly, then every single image was missing, which indicates + * that the dataset either had all empty values in column 2 or some other type of invalid + * string that should not prevent import. + */ + reject([ + 'CSV import was found to have a mix of missing images and images that were found', + 'in the data. This usually indicates a problem with the annotation file, but if', + 'you want to force the import to proceed, you can set all values in the', + 'Image Name column to be blank. Then DIVE will not attempt to validate image names.', + `Missing images include: ${missingImages.slice(0, 5)}...`, + ].join(' ')); } resolve({ tracks, fps }); }); @@ -296,8 +295,11 @@ async function parse(input: Readable, imageMap?: Map): Promise): Promise { - console.error(err); - reject(err); - }); + parser.on('error', reject); }); } diff --git a/server/dive_utils/serializers/viame.py b/server/dive_utils/serializers/viame.py index f4f3bd5e5..051891902 100644 --- a/server/dive_utils/serializers/viame.py +++ b/server/dive_utils/serializers/viame.py @@ -248,6 +248,7 @@ def load_csv_as_tracks_and_attributes( metadata_attributes: Dict[str, Dict[str, Any]] = {} test_vals: Dict[str, Dict[str, int]] = {} reordered = False + anyImageMatched = False missingImages: List[str] = [] for row in reader: if len(row) == 0 or row[0].startswith('#'): @@ -271,7 +272,10 @@ def load_csv_as_tracks_and_attributes( elif expectedFrameNumber is not feature.frame: # force reorder the annotations reordered = True + anyImageMatched = True feature.frame = expectedFrameNumber + else: + anyImageMatched = True if trackId not in tracks: tracks[trackId] = Track(begin=feature.frame, end=feature.frame, trackId=trackId) @@ -298,7 +302,7 @@ def load_csv_as_tracks_and_attributes( trackarr = tracks.items() - if imageMap and len(missingImages) and len(missingImages) != len(trackarr): + if imageMap and len(missingImages) and anyImageMatched: examples = ', '.join(missingImages[:3]) raise ValueError( ' '.join( From bbe295e05e3097d0c36c035104a3879033363c6a Mon Sep 17 00:00:00 2001 From: Brandon Davis Date: Tue, 1 Mar 2022 14:47:27 -0500 Subject: [PATCH 6/8] unified error handling for node versions --- .github/workflows/blank.yml | 4 ++-- .github/workflows/release.yml | 6 +++--- client/package.json | 2 +- .../platform/desktop/backend/serializers/viame.ts | 14 ++++++++++---- client/yarn.lock | 8 ++++---- 5 files changed, 20 insertions(+), 14 deletions(-) diff --git a/.github/workflows/blank.yml b/.github/workflows/blank.yml index 82b6d9a6e..266fedda3 100644 --- a/.github/workflows/blank.yml +++ b/.github/workflows/blank.yml @@ -19,9 +19,9 @@ jobs: - uses: actions/checkout@v2 - - uses: actions/setup-node@v1 + uses: actions/setup-node@v2 with: - node-version: '16.x' + node-version: '12.18.3' - name: Get yarn cache directory path id: yarn-cache-dir-path diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index d5f01bb6e..f7613d58d 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -22,10 +22,10 @@ jobs: # "ref" specifies the branch to check out. # "github.event.release.target_commitish" is a global variable and specifies the branch the release targeted ref: ${{ github.event.release.target_commitish }} - - name: Use Node 14 - uses: actions/setup-node@v1 + - name: Use Node 12 + uses: actions/setup-node@v2 with: - node-version: '16.x' + node-version: '12.18.3' # Specifies the registry, this field is required! registry-url: https://registry.npmjs.org/ - run: yarn install --frozen-lockfile diff --git a/client/package.json b/client/package.json index 78cf10aa2..726bf568a 100644 --- a/client/package.json +++ b/client/package.json @@ -76,7 +76,7 @@ "@types/mime-types": "^2.1.0", "@types/mock-fs": "^4.13.1", "@types/mousetrap": "^1.6.3", - "@types/node": "^14.0.5", + "@types/node": "^12.18.3", "@types/proper-lockfile": "^4.1.1", "@types/pump": "^1.1.0", "@types/range-parser": "^1.2.3", diff --git a/client/platform/desktop/backend/serializers/viame.ts b/client/platform/desktop/backend/serializers/viame.ts index ff122294a..b3f586f28 100644 --- a/client/platform/desktop/backend/serializers/viame.ts +++ b/client/platform/desktop/backend/serializers/viame.ts @@ -249,6 +249,7 @@ async function parse(input: Readable, imageMap?: Map): Promise((resolve, reject) => { pipeline([input, parser], (err) => { @@ -256,6 +257,9 @@ async function parse(input: Readable, imageMap?: Map): Promise 0 && anyImageMatched) { @@ -317,9 +321,10 @@ async function parse(input: Readable, imageMap?: Map): Promise): Promise Date: Tue, 1 Mar 2022 14:50:07 -0500 Subject: [PATCH 7/8] No continue --- client/platform/desktop/backend/serializers/viame.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/client/platform/desktop/backend/serializers/viame.ts b/client/platform/desktop/backend/serializers/viame.ts index b3f586f28..a9a9faded 100644 --- a/client/platform/desktop/backend/serializers/viame.ts +++ b/client/platform/desktop/backend/serializers/viame.ts @@ -324,6 +324,7 @@ async function parse(input: Readable, imageMap?: Map): Promise): Promise): Promise Date: Wed, 2 Mar 2022 09:13:57 -0500 Subject: [PATCH 8/8] Fix comment --- client/platform/desktop/backend/serializers/viame.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/platform/desktop/backend/serializers/viame.ts b/client/platform/desktop/backend/serializers/viame.ts index a9a9faded..b6eccf35d 100644 --- a/client/platform/desktop/backend/serializers/viame.ts +++ b/client/platform/desktop/backend/serializers/viame.ts @@ -264,11 +264,11 @@ async function parse(input: Readable, imageMap?: Map): Promise 0 && anyImageMatched) { /** - * If missing image count was different than track length, then some number of images + * If any image from CSV was not missing, then some number of images * from column 2 were actually valid and some were not. This indicates that the dataset * being loaded is probably corrupt. * - * If their counts match perfectly, then every single image was missing, which indicates + * If all images were missing, then every single image was missing, which indicates * that the dataset either had all empty values in column 2 or some other type of invalid * string that should not prevent import. */