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 support for frames-as-videos in nested groups #3935

Merged
merged 30 commits into from
Dec 20, 2023

Conversation

sashankaryal
Copy link
Contributor

@sashankaryal sashankaryal commented Dec 15, 2023

This PR adds support for nested groups in imavid looker.

To recreate the dataset in the following gif:

import fiftyone as fo
import fiftyone.zoo as foz
from fiftyone import ViewField as F

dynamic_groups_25 = fo.Dataset("dynamic-groups-25")

every_th_sample = 25
# for each slice, for every th samples, assign an arbitrary "scene_id"
for slice in dynamic_groups_25.group_slices:
    dynamic_groups_25.group_slice = slice
    scene_id_counter = 0
    order_by_counter = 0
    for sample in dynamic_groups_25:
        sample.set_field("scene_id", scene_id_counter // every_th_sample)
        sample.set_field("timestamp", order_by_counter % every_th_sample)
        sample.save()
        scene_id_counter = scene_id_counter + 1
        order_by_counter = order_by_counter + 1
dynamic_groups_25.save()

nested-group-small

@sashankaryal sashankaryal self-assigned this Dec 15, 2023
@sashankaryal sashankaryal requested a review from a team December 15, 2023 23:20
Copy link

codecov bot commented Dec 15, 2023

Codecov Report

Attention: 361 lines in your changes are missing coverage. Please review.

Comparison is base (f16c1f6) 15.85% compared to head (db8e7ba) 15.86%.
Report is 8 commits behind head on release/v0.23.2.

Files Patch % Lines
...p/packages/core/src/components/Actions/Options.tsx 0.00% 62 Missing ⚠️
app/packages/core/src/components/Modal/Modal.tsx 0.00% 54 Missing ⚠️
app/packages/state/src/recoil/groups.ts 41.55% 45 Missing ⚠️
app/packages/state/src/hooks/useCreateLooker.ts 0.00% 30 Missing ⚠️
...mponents/Actions/GroupMediaVisibilityContainer.tsx 0.00% 18 Missing ⚠️
.../src/components/Modal/Group/DynamicGroup/index.tsx 0.00% 18 Missing ⚠️
app/packages/state/src/recoil/selectors.ts 10.00% 18 Missing ⚠️
.../src/components/Sidebar/Entries/PathValueEntry.tsx 0.00% 15 Missing ⚠️
...nts/Modal/Group/DynamicGroup/NestedGroup/index.tsx 0.00% 14 Missing ⚠️
...p/packages/looker/src/lookers/imavid/controller.ts 0.00% 12 Missing ⚠️
... and 17 more
Additional details and impacted files
@@                Coverage Diff                @@
##           release/v0.23.2    #3935    +/-   ##
=================================================
  Coverage            15.85%   15.86%            
=================================================
  Files                  731      731            
  Lines                81636    81853   +217     
  Branches              1093     1093            
=================================================
+ Hits                 12946    12987    +41     
- Misses               68690    68866   +176     
Flag Coverage Δ
app 15.86% <15.25%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sashankaryal sashankaryal marked this pull request as ready for review December 19, 2023 17:33
Copy link
Contributor

@benjaminpkane benjaminpkane left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀 I think one edge case worth noting pcd slices have an unexpected behavior right now

Screen.Recording.2023-12-19.at.6.06.33.PM.mov

);

useEffect(() => {
// if it is unordered nested dynamic group and mode is not pagination, set to pagination
Copy link
Contributor

Choose a reason for hiding this comment

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

🚀

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Re: pcd slices, will fix it in next PR!

Thanks for taking a look, and making it better! 🥳

@sashankaryal sashankaryal merged commit 66bdfad into release/v0.23.2 Dec 20, 2023
12 of 13 checks passed
@sashankaryal sashankaryal deleted the feat/video-frames-groups branch December 20, 2023 00:14
@sashankaryal sashankaryal added feature Work on a feature request app Issues related to App features labels Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app Issues related to App features feature Work on a feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants