-
Notifications
You must be signed in to change notification settings - Fork 2
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
Conversation
…e videos in DLC directory).
…s in the table (selecting per page was buggy but it might be nice to implement)
…ataframe. pending ROI assignment from video metadata.
…in metadata tab table
Codecov Report
@@ 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
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this 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.
There was a problem hiding this 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:
- 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..
- can we have the suggested script download as a .py (or something) rather than a textbox.
Co-authored-by: Sam Cunliffe <[email protected]>
…eCentre/WAZP into add-dashboard-df-export. Incorporating changes from PR review.
There was a problem hiding this 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.
Conflicts! |
Change instructions to execute wazp.app as a module execution so we can have nice imports in app.py (allows for smoke test as well as the callbacks submodule. https://stackoverflow.com/questions/14132789/relative-imports-for-the-billionth-time
Added dataframe export functionality to the dashboard tab.
The user can export a subset of the available pose data by:
A dataframe is exported combining these two requirements. I added mock event tags to the sample project to check it works as intended.
Features:
Caveats
Pending bits: