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

Add dashboard df export #38

Merged
merged 41 commits into from
Mar 17, 2023
Merged

Add dashboard df export #38

merged 41 commits into from
Mar 17, 2023

Conversation

sfmig
Copy link
Collaborator

@sfmig sfmig commented Mar 1, 2023

Added dataframe export functionality to the dashboard tab.

The user can export a subset of the available pose data by:

  • selecting a subset of videos (for each of them there exists a corresponding h5 file with the pose data), and
  • selecting a subset of frames per video, based on the event tags.
    A dataframe is exported combining these two requirements. I added mock event tags to the sample project to check it works as intended.

Features:

  • If the pose data is not available for a video, the row will be automatically unselected and a message will be shown.
  • some extra columns are added to the exported dataframe, apart from those provided by DLC. These are 'event_tag' and 'ROI'.
    • 'Event_tag' assigns the 'event_tag' label to all frames from the instant in which the event is defined until a new event happens.
    • For now, the ROI label is just an empty string, but the idea would be to read the ROI path from the video metadata file and check if a bodypart at a specific frame is inside the ROI.
  • I added the 'Events' fields to the yaml file describing the metadata fields (otherwise the tests failed, thanks @samcunliffe 😇 ). To make it compatible with the table visualisation in the metadata tab, I modified it to show any field that is a dict ('Events' and 'ROI') as a string. I think for now it's ok, but we should make these columns not editable in the metadata tab.

Caveats

  • I'm exporting the dataframe using h5 files for now, but happy to consider other options going forward (mainly thinking about the corruption issues @niksirbi and @adamltyson mentioned a while ago).
  • it is a bit ambiguous to use the term 'event_tag' for an instant (i.e. a specific frame) and a period (in the exported dataframe). Should I use different terms?
  • the 'event_tag' in the exported dataframe is added for each bodypart. This is repetitive, since the event_tags refer only to the frame number and don't care about bodyparts, but I was struggling to get this done 'properly' with pandas. If anyone has pandas expertise I'm all ears, it has been driving me mad 😅 🐼 👎

Pending bits:

sfmig added 30 commits March 1, 2023 15:17
…s in the table (selecting per page was buggy but it might be nice to implement)
…ataframe. pending ROI assignment from video metadata.
@sfmig sfmig requested review from samcunliffe and niksirbi March 1, 2023 17:22
@codecov-commenter
Copy link

codecov-commenter commented Mar 1, 2023

Codecov Report

Merging #38 (b45cd8f) into main (9ae154c) will decrease coverage by 6.37%.
The diff coverage is 30.24%.

@@            Coverage Diff             @@
##             main      #38      +/-   ##
==========================================
- Coverage   40.39%   34.03%   -6.37%     
==========================================
  Files           9       11       +2     
  Lines         458      526      +68     
==========================================
- Hits          185      179       -6     
- Misses        273      347      +74     
Impacted Files Coverage Δ
wazp/pages/home.py 100.00% <ø> (ø)
wazp/utils.py 28.31% <8.69%> (-4.68%) ⬇️
wazp/callbacks/metadata.py 25.39% <25.39%> (ø)
wazp/callbacks/home.py 26.66% <26.66%> (ø)
wazp/callbacks/dashboard.py 28.37% <28.37%> (ø)
wazp/app.py 95.00% <100.00%> (+0.88%) ⬆️
wazp/callbacks/roi.py 16.86% <100.00%> (ø)
wazp/pages/04_dashboard.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

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

wazp/callbacks/metadata.py Outdated Show resolved Hide resolved
@sfmig sfmig marked this pull request as ready for review March 2, 2023 11:35
Copy link
Member

@niksirbi niksirbi left a comment

Choose a reason for hiding this comment

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

I found nothing critical.
I noticed that a fair bit of refactoring has crept into this PR (like dividing callbacks into separate files). This was sorely needed, but it may make it hard for you to merge into main now.

You may either choose to power through and resolve conflicts or try to transfer the refactoring bits to a separate PR. Whatever is easiest for you.

I would also remove the bits about the custom plots, since we are not going down that road after all.

wazp/callbacks/dashboard.py Outdated Show resolved Hide resolved
wazp/utils.py Show resolved Hide resolved
wazp/utils.py Outdated Show resolved Hide resolved
wazp/utils.py Outdated Show resolved Hide resolved
wazp/utils.py Outdated Show resolved Hide resolved
wazp/utils.py Show resolved Hide resolved
wazp/utils.py Show resolved Hide resolved
wazp/utils.py Show resolved Hide resolved
wazp/utils.py Outdated Show resolved Hide resolved
Copy link
Member

@samcunliffe samcunliffe left a comment

Choose a reason for hiding this comment

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

So it all looks good and the pandas.df export works 👍 .

My substantial comments are:

  1. if we really want to remove the auto plots from the dashboard (if we do this, we're not really a dashboard any more) and..
  2. can we have the suggested script download as a .py (or something) rather than a textbox.

wazp/pages/04_dashboard.py Show resolved Hide resolved
wazp/callbacks/dashboard.py Show resolved Hide resolved
wazp/callbacks/dashboard.py Outdated Show resolved Hide resolved
wazp/callbacks/dashboard.py Outdated Show resolved Hide resolved
wazp/app.py Outdated Show resolved Hide resolved
wazp/callbacks/dashboard.py Outdated Show resolved Hide resolved
wazp/callbacks/dashboard.py Show resolved Hide resolved
wazp/callbacks/dashboard.py Outdated Show resolved Hide resolved
wazp/utils.py Show resolved Hide resolved
wazp/utils.py Outdated Show resolved Hide resolved
@sfmig sfmig mentioned this pull request Mar 15, 2023
Copy link
Collaborator Author

@sfmig sfmig left a comment

Choose a reason for hiding this comment

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

Implemented the smaller fixes, and answered the larger requests with comments. I will take care of the bigger bits on separate PRs (following our short PR aims), but do let me know if you'd prefer otherwise.

wazp/callbacks/dashboard.py Outdated Show resolved Hide resolved
wazp/callbacks/dashboard.py Show resolved Hide resolved
wazp/callbacks/dashboard.py Outdated Show resolved Hide resolved
wazp/callbacks/metadata.py Outdated Show resolved Hide resolved
wazp/pages/04_dashboard.py Show resolved Hide resolved
wazp/callbacks/dashboard.py Show resolved Hide resolved
wazp/callbacks/dashboard.py Show resolved Hide resolved
wazp/callbacks/dashboard.py Show resolved Hide resolved
wazp/utils.py Show resolved Hide resolved
wazp/callbacks/dashboard.py Outdated Show resolved Hide resolved
@sfmig sfmig requested a review from samcunliffe March 16, 2023 10:35
@samcunliffe
Copy link
Member

Conflicts!

@samcunliffe samcunliffe mentioned this pull request Mar 16, 2023
1 task
@sfmig sfmig merged commit 0490d55 into main Mar 17, 2023
@sfmig sfmig deleted the add-dashboard-df-export branch March 17, 2023 10:09
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.

5 participants