Fix image map loading logic on desktop#1190
Conversation
|
This PR allows all non-empty invalid strings instead of strictly how it was before with image paths vs blank?
So your annotations could still be out of order if you mess up the relative/full paths for the images. I'm not proposing that we fix this corner case, just trying to make sure I understand what is going on. |
Yeah.... I don't really know how to juggle all the conflicting needs here, and this seemed like the best option. As I recall, the main motivation behind reordering is to un-mangle out of order annotations when someone provides the images, and not really to defend against bad data entering the system. It's more of a "you met a special case that lets us help you" feature. Because we need to accept so many invalid types of data ( |
|
I'm not suggesting we error or prevent having timestamps in that field. I just wanted to understand the possible failure conditions of this change. |
BryonLewis
left a comment
BryonLewis
left a comment
There was a problem hiding this comment.
Handle the case of multi-frame tracks that have all missing images. This would fail for multiple detections per frame, or multi-frame tracks with the tracks.length being equal to the number of missing images. I think you really want a frame counter and determine that the number of frames in the CSV is equal to the missingImages.length.
|
|
||
| trackarr = tracks.items() | ||
|
|
||
| if imageMap and len(missingImages) and len(missingImages) != len(trackarr): |
There was a problem hiding this comment.
isn't this assuming you are only dealing with detections and not tracks?
add in a test probably for tracks that have all missingImages.
| const tracks = Array.from(dataMap.values()); | ||
|
|
||
| if (imageMap !== undefined) { | ||
| if (missingImages.length > 0 && missingImages.length !== tracks.length) { |
There was a problem hiding this comment.
Same as other comment. There is no relationship between the number of tracks and the missingImages, unless there is a single detection per frame. This would fail for image-sequences composed of multiframe tracks that don't have filenames. Like on the python version I'd suggest we modify the tests to account for that as well.
|
Warning: I had a locally installed version of Nodejs with a buggy implementation of |
BryonLewis
left a comment
BryonLewis
left a comment
There was a problem hiding this comment.
Did some testing and looks good, update the documentation and should be good.
| * 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. |
There was a problem hiding this comment.
Update this documentation and it should be good to go.
2 participants
Approach is explained in a large comment block in
viame.tsfixes #1167