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

Update status message on status bar #1411

Merged
merged 7 commits into from
Jul 31, 2023

Conversation

shrivaths16
Copy link
Contributor

@shrivaths16 shrivaths16 commented Jul 25, 2023

Description

While removing videos from the gui, the status bar at the bottom does not update correctly. Now trying to update it properly after every video is being removed.

Related PR:

Types of changes

  • Bugfix
  • New feature
  • Refactor / Code style update (no logical changes)
  • Build / CI changes
  • Documentation Update
  • Other (explain)

Does this address any currently open issues?

Outside contributors checklist

  • Review the guidelines for contributing to this repository
  • Read and sign the CLA and add yourself to the authors list
  • Make sure you are making a pull request against the develop branch (not main). Also you should start your branch off develop
  • Add tests that prove your fix is effective or that your feature works
  • Add necessary documentation (if appropriate)

Thank you for contributing to SLEAP!

❤️

@talmo
Copy link
Collaborator

talmo commented Jul 27, 2023

Looks like we shouldn't be trying to query for the number of labeled frames for a video that does not exist:

Traceback (most recent call last):
  File "/home/runner/work/sleap/sleap/sleap/gui/app.py", line 1227, in _after_plot_change
    self.updateStatusMessage()
  File "/home/runner/work/sleap/sleap/sleap/gui/app.py", line 1270, in updateStatusMessage
    current_video, "predicted"
  File "/home/runner/work/sleap/sleap/sleap/io/dataset.py", line [107](https://github.com/talmolab/sleap/actions/runs/5661452334/job/15339366532?pr=1411#step:5:108)3, in get_labeled_frame_count
    return self._cache.get_frame_count(video, filter)
  File "/home/runner/work/sleap/sleap/sleap/io/dataset.py", line 289, in get_frame_count
    video, filter
  File "/home/runner/work/sleap/sleap/sleap/io/dataset.py", line 319, in get_filtered_frame_idxs
    video_idx = self.labels.videos.index(video)
ValueError: Video(backend=MediaVideo(filename='tests/data/videos/centered_pair_small.mp4', grayscale=True, bgr=True, dataset='', input_format='')) is not in list

Namely, in updateStatusMessage. Maybe we should grab it from state["video"]?

If state["video"] is still set to a video that does not exist, then it really should be set to None. If this is being handled elsewhere, it's possible that this callback is being triggered before state["video"] is appropriately updated to None. We should check why the RemoveVideo command isn't doing this, and if it's supposed to be, why it's not applied when this callback is called.

A quick patch would be to check for whether current_video in self.labels.videos, and if False, just bypass checking for frame count. That said, it would be preferable to find out why this workaround is necessary in the first place.

More hints:

@codecov
Copy link

codecov bot commented Jul 27, 2023

Codecov Report

Merging #1411 (52a27a2) into develop (3a01ef3) will increase coverage by 0.00%.
The diff coverage is 91.66%.

@@           Coverage Diff            @@
##           develop    #1411   +/-   ##
========================================
  Coverage    73.02%   73.02%           
========================================
  Files          133      133           
  Lines        23752    23757    +5     
========================================
+ Hits         17345    17349    +4     
- Misses        6407     6408    +1     
Files Changed Coverage Δ
sleap/gui/commands.py 61.42% <83.33%> (-0.02%) ⬇️
sleap/gui/app.py 75.31% <100.00%> (+0.03%) ⬆️
sleap/io/dataset.py 83.86% <100.00%> (+0.03%) ⬆️

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

@roomrys roomrys mentioned this pull request Jul 28, 2023
11 tasks
@roomrys
Copy link
Collaborator

roomrys commented Jul 28, 2023

#1422 should resolve the strange error we were seeing in these tests. I don't think we need the extra "conditional state check" code here from the second commit.

@shrivaths16 shrivaths16 marked this pull request as ready for review July 31, 2023 19:42
@shrivaths16 shrivaths16 requested a review from talmo July 31, 2023 20:01
Copy link
Collaborator

@talmo talmo left a comment

Choose a reason for hiding this comment

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

LGTM! Good catches!

@talmo talmo merged commit 6002332 into develop Jul 31, 2023
9 checks passed
roomrys pushed a commit that referenced this pull request Sep 10, 2023
commit 734283a
Author: Liezl Maree <38435167+roomrys@users.noreply.github.com>
Date:   Sun Sep 10 08:19:36 2023 -0700

    Bump to 1.3.2 (#1482)

    * Bump to 1.3.2

    * Run manual build w/o pip upload

    * Use `importlib.resources` if available

    * Rebuild mac conda package

    * Lint

    * Build/upload pip wheel

    * Reset build_manual and up build number

commit e424501
Author: Liezl Maree <38435167+roomrys@users.noreply.github.com>
Date:   Sat Sep 9 10:32:29 2023 -0700

    Add pip extras (#1481)

    * Rename pip to pypi, add tensorflow

    * Move out jupyter requirements from dev

    * Add extras for conda/pip jupyter and dev

    * Rename `pip` extra to `pypi`

    * Add build_ci workflow

    * Install pip package using extras

    * Rerun notebooks with new pip wheel

    * Internal import after adding relative path to sys

    * Build develop docs on this branch

    * Add comments to setup.py extras

    * Update installation docs

    * Add wget bypass for apple silicon mambaforge

    * Create func to combine req

    * Italicize jupyter instead of bold

    ---------

    Co-authored-by: modularizer <modularizer@gmail.com>
    Co-authored-by: roomrys <>

commit 93ef288
Author: Shrivaths Shyam <52810689+shrivaths16@users.noreply.github.com>
Date:   Sat Sep 9 03:12:58 2023 +0530

    Limit max tracks via track-local queues (#1447)

    * Initial commit

    * format files

    * [wip] adding local deque for tracks

    * format files

    * [wip] adding local deque for tracks

    * [wip] Add max tracking for simpletracker

    * [wip] Add max tracking for simple tracker

    * [wip] add missing argument

    * [wip] Add and modify test functions

    * [wip] Add and modify test functions

    * Bug fix and refactoring code

    * [wip] Add max tracking for flow tracker.

    * [wip] Including suggested changes

    * [wip] refactor code

    * Add test function to check max tracks

    * Added suggestions and feedback

    * Prevent the creation of more than max tracks when we have unmatched detections

    * Add tests

    * Use maximum tracking by default when loading model via high level API

    * Lint

    * Fix integration test

    * Refactor max tracker tests

    * Add integration test for CLI

    * typo

    * Add max tracks to the tracking GUI

    * Update CLI docs and add examples

    ---------

    Co-authored-by: Talmo Pereira <talmo@princeton.edu>
    Co-authored-by: Talmo Pereira <talmo@salk.edu>

commit 64655d6
Author: DivyaSesh <64513125+gitttt-1234@users.noreply.github.com>
Date:   Wed Sep 6 09:50:35 2023 -0700

    Fix Auto-select GPU (#1474)

    * Fix Auto-select GPU

    * Format file

    * Add variable in init

    * Format files

    * Add small test to ensure environment variable is set

    * Make linter happy

    ---------

    Co-authored-by: roomrys <lmaree@ucsd.edu>

commit 0ef52cd
Author: Liezl Maree <38435167+roomrys@users.noreply.github.com>
Date:   Thu Aug 17 11:03:51 2023 -0700

    Migrate to `importlib_resources` backport (#1458)

    * Switch to  backport

    * Remove `pkg_resources`

    * Clean-up function (non-logical)

    * Make linter happy

    * Fix-up path for last few stragglers

    * Use `Path.as_posix` method instead of `str`

commit e0eebb2
Author: Liezl Maree <38435167+roomrys@users.noreply.github.com>
Date:   Tue Aug 15 13:10:06 2023 -0700

    Handle error message edge case when finding yaml paths (#1456)

commit 6858563
Author: Liezl Maree <38435167+roomrys@users.noreply.github.com>
Date:   Fri Aug 11 11:33:14 2023 -0700

    Add message if drag drop fails (#1451)

    * Fix drag and drop

    * Feedback when user drops invalid file type

    * Change wording on message

commit 88fdb68
Author: Liezl Maree <38435167+roomrys@users.noreply.github.com>
Date:   Fri Aug 11 11:27:57 2023 -0700

    Pin `tensorflow-hub<0.14.0` (#1446)

    * Pin pynwb 2.3.3

    * Remove pynwb pin, add comments

commit 4730788
Author: Talmo Pereira <talmo@salk.edu>
Date:   Fri Aug 11 10:52:09 2023 -0700

    Fix drag and drop (#1449)

commit 47f8096
Author: DivyaSesh <64513125+gitttt-1234@users.noreply.github.com>
Date:   Thu Aug 10 16:29:39 2023 -0700

    Add Option to Export CSV (#1438)

    * Add Option to Export CSV

    * Add Test Functions

    * Fomat Files

    * Change FormatID

commit 5ba6bc1
Author: Liezl Maree <38435167+roomrys@users.noreply.github.com>
Date:   Thu Aug 10 10:17:45 2023 -0700

    Add model folder to the unzip path (#1445)

    * Add model folder to the unzip path

    * Handle cases where zipped model either has no extra directory

    * Add test

    * Fix-up test and implementation

    * Manually lint

commit d61a184
Author: KevinZ0217 <96039456+KevinZ0217@users.noreply.github.com>
Date:   Wed Aug 9 14:47:21 2023 -0700

    Change the hotkey for exporting h5 analysis (#1444)

    * Change the hotkey for export h5 files to Ctrl+Alt+E

commit ad7529e
Author: KevinZ0217 <96039456+KevinZ0217@users.noreply.github.com>
Date:   Wed Aug 9 14:46:09 2023 -0700

    Add a button for copying model config to clipboard (#1433)

    * Add shortcur for export_analysis_current

    * Fix the linting issue

    * Add the button without copy method'

    * Add button for copying model config to clipboard

    * Fix linting by reformatting

    * Use Qtpy for clipboard rather than pyperclip

    * Pretty print model config json to clipboard and fix missing command

    * Fix the overwriting problem for dict object

    * Delete unnecessary print statement

    * Add a few comments & Remove unnecessary variables & remove unused function

commit 2611e7d
Author: Liezl Maree <38435167+roomrys@users.noreply.github.com>
Date:   Wed Aug 9 11:52:57 2023 -0700

    Improve error message for detecting video backend (#1441)

    * Improve error message for detecting video backend

    * Lint

    * Add small test

commit 1151a95
Author: Shrivaths Shyam <52810689+shrivaths16@users.noreply.github.com>
Date:   Wed Aug 2 17:08:38 2023 -0700

    Fix error thrown when last video is deleted  (#1421)

    * Handle None Video case during callbacks

    * format files

    * remove unused comments

    * Disable remove video button when there are no videos

    * Display default background when all videos are removed

    * Format files

    * Remove overlay error after removing last video

    * Redraw overlays on plot change (#1435)

    * Redraw overlays after changedPlot, changedPlot on reset

    * Update instance state on player reset

    ---------

    Co-authored-by: Liezl Maree <38435167+roomrys@users.noreply.github.com>

commit 6002332
Author: Shrivaths Shyam <52810689+shrivaths16@users.noreply.github.com>
Date:   Mon Jul 31 13:28:53 2023 -0700

    Update status message on status bar (#1411)

    * Update status message on status bar

    * Update statusbar to show correct video count

    * remove additional conditional check

commit 3a01ef3
Author: Liezl Maree <38435167+roomrys@users.noreply.github.com>
Date:   Mon Jul 31 12:09:50 2023 -0700

    Correct GUI state emulation (#1422)

commit e94b516
Author: Liezl Maree <38435167+roomrys@users.noreply.github.com>
Date:   Thu Jul 27 12:11:41 2023 -0700

    Add video path and frame indices to metrics (#1396)

    * Add `Instance`s and `PredictedInstance`s to metrics

    * Add tests

    * Add frame/video info to metrics, wip: test writing

    * Fix metrics save test

commit b2ad203
Author: Liezl Maree <38435167+roomrys@users.noreply.github.com>
Date:   Thu Jul 27 12:08:19 2023 -0700

    Fix labels export for json (#1410)

    * wip: fix labels export for json

    * Add test for json.zip labels pkg

    * Add test for .slp labels pkg

    * Make linter happy

commit 90c012d
Author: KevinZ0217 <96039456+KevinZ0217@users.noreply.github.com>
Date:   Wed Jul 26 13:30:22 2023 -0700

    Add shortcut to export analysis for current video (#1414)

    * Add shortcur for export_analysis_current

    * Fix the linting issue

commit f9d0a22
Author: Shrivaths Shyam <52810689+shrivaths16@users.noreply.github.com>
Date:   Tue Jul 25 09:37:52 2023 -0700

    Modify compute OKS function (#1399)

    * Update compute OKS function

    * Update compute OKS function

    * Modify compute OKS function

    * Added suggested changes

    * Added further suggestions and comments

    * Added the permalink to the cocoeval function

    * Added permalink to cocoeval function

    ---------

    Co-authored-by: Liezl Maree <38435167+roomrys@users.noreply.github.com>
    Co-authored-by: Talmo Pereira <talmo@salk.edu>

commit 904338c
Author: Liezl Maree <38435167+roomrys@users.noreply.github.com>
Date:   Tue Jul 25 06:34:25 2023 -0700

    Add `Video` to cache when adding `Track` (#1407)

    * Add `Video` to cache when adding `Track`

    * Use methods instead of rewriting code

    * Simplify code

commit 845214c
Author: DivyaSesh <64513125+gitttt-1234@users.noreply.github.com>
Date:   Mon Jul 24 21:20:02 2023 -0700

    Fix Remove Videos in Batch (#1406)

    * Fix Remove Videos in Batch

    * Remove Unused Testing Code

commit 0afbb9b
Author: Liezl Maree <38435167+roomrys@users.noreply.github.com>
Date:   Mon Jul 24 17:37:42 2023 -0700

    Add `Track` when add `Instance` (#1408)

commit d173303
Author: Liezl Maree <38435167+roomrys@users.noreply.github.com>
Date:   Mon Jul 24 08:25:09 2023 -0700

    Fix skeleton templates (#1404)

commit 0e7a372
Author: DivyaSesh <64513125+gitttt-1234@users.noreply.github.com>
Date:   Fri Jul 21 09:49:51 2023 -0700

    Fix panning bounding box (#1398)

commit fb61b6c
Author: Liezl Maree <38435167+roomrys@users.noreply.github.com>
Date:   Wed Jul 19 15:30:43 2023 -0700

    Fix `Filedialog` to work across (mac)OS (#1393)

    * Always use dir instead of directory

    * Wrap `FileDialog` methods for OS-specific calls

    * Clean-up os-specific wrapper to check for linux only

    * Lint

    * Fix test for non native `FileDialog`

commit 19cd2b5
Author: DivyaSesh <64513125+gitttt-1234@users.noreply.github.com>
Date:   Mon Jul 17 17:55:29 2023 -0700

    Add option to remove videos in batch (#1382)

    * add option to remove videos in batch

    * Add option to remove videos in batch

    * Add option to remove videos in batches

    * Modify Lint format

    * Add Test cases

    * Modify Test Cases for Removing Videos in Batch

    * Add Comment to test_docks

    * Remove commented line

    * Format files

commit 3b5c4ff
Author: Shrivaths Shyam <52810689+shrivaths16@users.noreply.github.com>
Date:   Fri Jul 14 17:06:58 2023 -0700

    Minor fix in computation of OKS (#1383)

    * fix compute oks

    * Update the oks fix

commit 1c2be11
Author: Liezl Maree <38435167+roomrys@users.noreply.github.com>
Date:   Thu Jul 6 14:44:01 2023 -0700

    Pin micromamba version (#1376)

    * Pin micromamba version

    * Add build number to pin
@roomrys roomrys deleted the shrivaths/update-status-on-remove-video branch September 11, 2023 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants