ROI additions

[niksirbi]

Closes #44

[niksirbi]

Changes

• Change layout of elements on the ROI tab to more closely reflect the order of actions taken by the user
• Add ability to multi-select rows of the ROI table and delete them (and the corresponding shapes) via a Delete selected button
• The ROI buttons (save, load, delete, infer) are disabled by default, and are only enabled when it makes sense (i.e. when there is something to save/load, or when there is sth selected for deletion.
• More informative messages to the user, depending on both ROIs defined in the app and on those saved to the file.

[codecov-commenter]

Codecov Report

Merging #64 (37d000e) into main (cf601c0) will decrease coverage by 0.30%.
The diff coverage is 17.72%.

@@            Coverage Diff             @@
##             main      #64      +/-   ##
==========================================
- Coverage   32.99%   32.69%   -0.30%     
==========================================
  Files          12       12              
  Lines         585      624      +39     
==========================================
+ Hits          193      204      +11     
- Misses        392      420      +28     

Impacted Files	Coverage Δ
wazp/utils.py	23.07% <0.00%> (ø)
wazp/callbacks/roi.py	17.78% <12.50%> (+0.92%)	⬆️
wazp/pages/02_ROI.py	100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[niksirbi]

The new layout for the ROI tab:

[sfmig]

Looks good! I think with the new layout it improved a lot ✨
I just added minor suggestions, mostly to make alert messages and displayed texts more concise.

One bigger suggestion I made is to remove the instructions. I think with the new layout it should be clear enough (we can get feedback from Sanna on this and put them back if it's not clear enough). Another one is a potential issue with the frame_step calculation (I think it becomes 0 if the video has less than 2004 frames).

[sfmig]

above in line 177 (I cannot comment on lines without changes):

frame_step becomes 0 if (num_frames / 4) < 500 . Could this be a problem?

We may have videos with less than 2000 frames right? (actually the limit is 2004 frames because it's wrapped in an int).

I like the slider option, but an alternative to avoid issues with steps may be to just show the user the total number of frames and let them pick one from 0 to the max number of frames. Maybe something like showing [ ] / 2500, where [ ] is an editable input text box, and by default [ ] is populated with the midpoint of the video. Then we can have buttons for +1, -1, +10, -10 in case that frame is not convenient to quickly switch but stay in the vicinity.

If you feel this is a big change for v0, feel free to add as an enhancement issue

[sfmig]

actually I think this may be the case in this video? note the -2 in the slider

[sfmig]

mmm I tried again and it showed 294k frames so not sure... 😬

[sfmig]

I was just checking on a new video and I found this:

when I define ROIs for a video with no ROIs defined, and then click 'Save all', the message doesn't update to show they were successfully saved (it stays with 'detected unsaved changes'). They do save correctly though.

I also get an error:

Traceback (most recent call last):
  File "/Users/sofia/Documents_local/project_Zoo_SWC/WAZP/wazp/callbacks/roi.py", line 498, in update_frame_graph
    next_shape_color = roi_color_mapping["roi2color"][roi_name]
KeyError: 'roi2color'

[niksirbi]

frame_step becomes 0 if (num_frames / 4) < 500 . Could this be a problem?
We may have videos with less than 2000 frames right? (actually the limit is 2004 frames because it's wrapped in an int).

Good catch! I now do the rounding to nearest 1000 only if the frame step is >1000.

# Divide the number of frames into 4 steps
frame_step = int(num_frames / 4)
# Round to the nearest 1000 if step is > 1000
if frame_step > 1000:
    frame_step = int(frame_step / 1000) * 1000

Therefore, if num_frames < 4004, no rounding will occur. I suspect most videos will be many thousands of frames, but it's good to be robust to edge cases.

Apart from that, under the frame I now display "Showing frame n/N", making things more explicit.

I like the slider option, but an alternative to avoid issues with steps may be to just show the user the total number of frames and let them pick one from 0 to the max number of frames. Maybe something like showing [ ] / 2500, where [ ] is an editable input text box, and by default [ ] is populated with the midpoint of the video. Then we can have buttons for +1, -1, +10, -10 in case that frame is not convenient to quickly switch but stay in the vicinity.

I opened this as a separate issue #71

[niksirbi]

actually I think this may be the case in this video? note the -2 in the slider

That is in fact a separate problem, and it sent me down a rabbit hole yesterday. So if the video cannot be read (e.g. if not linked via alias), OpenCV still tries to calculate frames and fails. However, that error was not properly caught and you end up with negative numbers sometimes. This is further complicated by the frame caching locally, because sometimes even if the video itself cannot be read, the app still displays the locally cached frame (from a previous session).

Anyhow, to make things less confusing and much more explicit, I now throw an error when the video cannot be read and raise an alert in the app. This was not very straightforward, because catching OpenCV exceptions in Python does not work smoothly (despite an existing cv2.error object which is supposed to do this job). Therefore I had to come up with some indirect ways of figuring out when the video loading had gone wrong. I now include these checks in the utils.get_num_frames and utils.extract_frame functions directly.

[niksirbi]

when I define ROIs for a video with no ROIs defined, and then click 'Save all', the message doesn't update to show they were successfully saved (it stays with 'detected unsaved changes'). They do save correctly though.

Great catch! That one is entirely my fault, flawed logic. I now know why this happens and will fix it.

[sfmig]

nice workaround! ✨

[niksirbi]

@sfmig thanks again for all the comments, I've fixed most of the issues you found (I think).
Would you ming giving it another test now?

[sfmig]

LGTM!