From b6041a903adaf9834f050590e05e1a82e65927a3 Mon Sep 17 00:00:00 2001 From: manivoxel51 Date: Wed, 29 Nov 2023 10:59:37 -0800 Subject: [PATCH 1/2] selectGroupSlices run before select samples --- .../core/src/components/Actions/utils.tsx | 1 + .../looker/src/elements/common/controls.ts | 1 - .../state/src/hooks/useSetExpandedSample.ts | 2 + app/packages/state/src/recoil/modal.ts | 2 + fiftyone/core/stages.py | 6 +++ fiftyone/core/view.py | 4 ++ fiftyone/server/view.py | 1 - tests/unittests/view_tests.py | 50 ++++++++++++------- 8 files changed, 48 insertions(+), 19 deletions(-) diff --git a/app/packages/core/src/components/Actions/utils.tsx b/app/packages/core/src/components/Actions/utils.tsx index ae586271df..213075c283 100644 --- a/app/packages/core/src/components/Actions/utils.tsx +++ b/app/packages/core/src/components/Actions/utils.tsx @@ -77,6 +77,7 @@ export const tagStatistics = selectorFamily< ? { id: modal ? get(groupId) : null, slices: get(fos.currentSlices(modal)), + slice: get(fos.currentSlice(modal)), mode: get(groupStatistics(modal)), } : null, diff --git a/app/packages/looker/src/elements/common/controls.ts b/app/packages/looker/src/elements/common/controls.ts index d46873559b..2343abaa2a 100644 --- a/app/packages/looker/src/elements/common/controls.ts +++ b/app/packages/looker/src/elements/common/controls.ts @@ -66,7 +66,6 @@ export class ControlsElement< error, loaded, }: Readonly) { - console.log("showcontrols is ", showControls); showControls = showControls && !disableControls && !error && loaded; if (this.showControls === showControls) { return this.element; diff --git a/app/packages/state/src/hooks/useSetExpandedSample.ts b/app/packages/state/src/hooks/useSetExpandedSample.ts index 550d9990e5..3e21ee2eae 100644 --- a/app/packages/state/src/hooks/useSetExpandedSample.ts +++ b/app/packages/state/src/hooks/useSetExpandedSample.ts @@ -27,6 +27,7 @@ export default () => { ) => { set(groupAtoms.groupId, groupId || null); set(currentModalSample, { id, index }); + reset(groupAtoms.dynamicGroupIndex); reset(dynamicGroupCurrentElementIndex); groupByFieldValue && @@ -48,6 +49,7 @@ export default () => { ({ snapshot }) => async (index: number | ((current: number) => number)) => { const current = await snapshot.getPromise(currentModalSample); + if (index instanceof Function) { index = index(current.index); } diff --git a/app/packages/state/src/recoil/modal.ts b/app/packages/state/src/recoil/modal.ts index 1f5083af76..750571cfbc 100644 --- a/app/packages/state/src/recoil/modal.ts +++ b/app/packages/state/src/recoil/modal.ts @@ -21,6 +21,7 @@ import { RelayEnvironmentKey } from "./relay"; import { datasetName } from "./selectors"; import { mapSampleResponse } from "./utils"; import { view } from "./view"; +import { modal } from "./atoms"; export const modalLooker = atom | null>({ key: "modalLooker", @@ -180,6 +181,7 @@ export const modalSample = graphQLSelector< }, variables: ({ get }) => { const current = get(currentModalSample); + if (current === null) return null; const slice = get(groupSlice); diff --git a/fiftyone/core/stages.py b/fiftyone/core/stages.py index af2250e470..90572fa29c 100644 --- a/fiftyone/core/stages.py +++ b/fiftyone/core/stages.py @@ -8504,6 +8504,12 @@ def repr_ViewExpression(self, expr, level): MatchTags, Select, SelectBy, + SelectGroupSlices, Skip, Take, } + +# Registry of select stages that should select first +_STAGES_THAT_SELECT_FIRST = { + SelectGroupSlices, +} diff --git a/fiftyone/core/view.py b/fiftyone/core/view.py index 557e099fd3..c9da51250b 100644 --- a/fiftyone/core/view.py +++ b/fiftyone/core/view.py @@ -1820,6 +1820,10 @@ def make_optimized_select_view( else: if view.media_type == fom.GROUP and view.group_slices and flatten: view = view.select_group_slices(_allow_mixed=True) + else: + for stage in stages: + if type(stage) in fost._STAGES_THAT_SELECT_FIRST: + view = view._add_view_stage(stage, validate=False) view = view.select(sample_ids, ordered=ordered) diff --git a/fiftyone/server/view.py b/fiftyone/server/view.py index da665d6464..ff33ad8d94 100644 --- a/fiftyone/server/view.py +++ b/fiftyone/server/view.py @@ -144,7 +144,6 @@ def get_view( pagination_data=pagination_data, extended_stages=extended_stages, ) - return view diff --git a/tests/unittests/view_tests.py b/tests/unittests/view_tests.py index 872a00c477..7d0f3511d3 100644 --- a/tests/unittests/view_tests.py +++ b/tests/unittests/view_tests.py @@ -4257,23 +4257,7 @@ def test_view_field_copy(self): self.assertEqual(str(field), str(deepcopy(field))) def test_make_optimized_select_view_group_dataset(self): - dataset = fo.Dataset() - dataset.add_group_field("group", default="center") - - groups = ["left", "center", "right"] - filepaths = [ - [str(i) + str(j) + ".jpg" for i in groups] for j in range(3) - ] - filepaths = [dict(zip(groups, fps)) for fps in zip(*filepaths)] - group = fo.Group() - samples = [] - for fps in filepaths: - for name, filepath in fps.items(): - samples.append( - fo.Sample(filepath=filepath, group=group.element(name)) - ) - - sample_ids = dataset.add_samples(samples) + dataset, sample_ids = self._make_group_dataset() optimized_view = fov.make_optimized_select_view( dataset, sample_ids[0], flatten=True @@ -4284,6 +4268,22 @@ def test_make_optimized_select_view_group_dataset(self): ] self.assertEqual(optimized_view._all_stages, expected_stages) + def test_make_optimized_select_view_select_group_slices_before_sample_selection( + self, + ): + dataset, sample_ids = self._make_group_dataset() + view = dataset.select_group_slices(["left", "right"]) + + optimized_view = fov.make_optimized_select_view( + view, + sample_ids[1], + ) + + first_stage, second_stage = optimized_view._stages + # the order matters + self.assertEqual(type(first_stage), fosg.SelectGroupSlices) + self.assertEqual(type(second_stage), fosg.Select) + def test_selected_samples_in_group_slices(self): (dataset, selected_ids) = self._make_group_by_group_dataset() view = dataset.view() @@ -4307,6 +4307,22 @@ def test_selected_samples_in_group_slices(self): ) self.assertEqual(len(optimized_view), 2) + def _make_group_dataset(self): + dataset = fo.Dataset() + dataset.add_group_field("group", default="left") + groups = ["left", "right"] + filepaths = [str(i) + ".jpg" for i in groups] + + filepaths = [dict(zip(groups, fps)) for fps in zip(*filepaths)] + group = fo.Group() + samples = [] + for fps in filepaths: + for name, filepath in fps.items(): + samples.append( + fo.Sample(filepath=filepath, group=group.element(name)) + ) + return dataset, dataset.add_samples(samples) + def _make_group_by_group_dataset(self): dataset = fo.Dataset() dataset.add_group_field("group_field", default="left") From 0b010e992ff850ed24f676f7c92a5971229057fb Mon Sep 17 00:00:00 2001 From: manivoxel51 Date: Wed, 29 Nov 2023 11:18:27 -0800 Subject: [PATCH 2/2] remove unused import --- app/packages/state/src/hooks/useSetExpandedSample.ts | 1 - app/packages/state/src/recoil/modal.ts | 2 -- 2 files changed, 3 deletions(-) diff --git a/app/packages/state/src/hooks/useSetExpandedSample.ts b/app/packages/state/src/hooks/useSetExpandedSample.ts index 3e21ee2eae..0257885f28 100644 --- a/app/packages/state/src/hooks/useSetExpandedSample.ts +++ b/app/packages/state/src/hooks/useSetExpandedSample.ts @@ -49,7 +49,6 @@ export default () => { ({ snapshot }) => async (index: number | ((current: number) => number)) => { const current = await snapshot.getPromise(currentModalSample); - if (index instanceof Function) { index = index(current.index); } diff --git a/app/packages/state/src/recoil/modal.ts b/app/packages/state/src/recoil/modal.ts index 750571cfbc..675a83259a 100644 --- a/app/packages/state/src/recoil/modal.ts +++ b/app/packages/state/src/recoil/modal.ts @@ -21,7 +21,6 @@ import { RelayEnvironmentKey } from "./relay"; import { datasetName } from "./selectors"; import { mapSampleResponse } from "./utils"; import { view } from "./view"; -import { modal } from "./atoms"; export const modalLooker = atom | null>({ key: "modalLooker", @@ -122,7 +121,6 @@ export const modalSampleIndex = selector({ key: "modalSampleIndex", get: ({ get }) => { const current = get(currentModalSample); - if (!current) { throw new Error("modal sample is not defined"); }