From 6ea76d61e42874e391a17d1067d7e8bc932346e1 Mon Sep 17 00:00:00 2001 From: imanjra Date: Thu, 26 Sep 2024 10:20:24 -0400 Subject: [PATCH 1/4] add group slice in operator context --- app/packages/operators/src/CustomPanel.tsx | 2 ++ app/packages/operators/src/hooks.ts | 3 +++ app/packages/operators/src/operators.ts | 9 +++++++++ app/packages/operators/src/state.ts | 5 +++++ app/packages/operators/src/useCustomPanelHooks.ts | 7 +++++++ docs/source/plugins/developing_plugins.rst | 13 +++++++++++++ fiftyone/operators/executor.py | 5 +++++ fiftyone/operators/operations.py | 3 +++ fiftyone/operators/panel.py | 1 + 9 files changed, 48 insertions(+) diff --git a/app/packages/operators/src/CustomPanel.tsx b/app/packages/operators/src/CustomPanel.tsx index 8c7c1e07e9..cf9f82a5b3 100644 --- a/app/packages/operators/src/CustomPanel.tsx +++ b/app/packages/operators/src/CustomPanel.tsx @@ -114,6 +114,7 @@ export function defineCustomPanel({ on_change_selected, on_change_selected_labels, on_change_extended_selection, + on_change_group_slice, panel_name, panel_label, }) { @@ -130,6 +131,7 @@ export function defineCustomPanel({ onChangeSelected={on_change_selected} onChangeSelectedLabels={on_change_selected_labels} onChangeExtendedSelection={on_change_extended_selection} + onChangeGroupSlice={on_change_group_slice} dimensions={dimensions} panelName={panel_name} panelLabel={panel_label} diff --git a/app/packages/operators/src/hooks.ts b/app/packages/operators/src/hooks.ts index 5eac79790c..a636f1b263 100644 --- a/app/packages/operators/src/hooks.ts +++ b/app/packages/operators/src/hooks.ts @@ -25,6 +25,7 @@ function useOperatorThrottledContextSetter() { const filters = useRecoilValue(fos.filters); const selectedSamples = useRecoilValue(fos.selectedSamples); const selectedLabels = useRecoilValue(fos.selectedLabels); + const groupSlice = useRecoilValue(fos.groupSlice); const currentSample = useCurrentSample(); const setContext = useSetRecoilState(operatorThrottledContext); const setThrottledContext = useMemo(() => { @@ -47,6 +48,7 @@ function useOperatorThrottledContextSetter() { selectedLabels, currentSample, viewName, + groupSlice, }); }, [ setThrottledContext, @@ -58,6 +60,7 @@ function useOperatorThrottledContextSetter() { selectedLabels, currentSample, viewName, + groupSlice, ]); } diff --git a/app/packages/operators/src/operators.ts b/app/packages/operators/src/operators.ts index 621554d47c..f11aa3d880 100644 --- a/app/packages/operators/src/operators.ts +++ b/app/packages/operators/src/operators.ts @@ -90,6 +90,7 @@ export type RawContext = { selection: string[] | null; scope: string; }; + groupSlice: string; }; export class ExecutionContext { @@ -132,6 +133,9 @@ export class ExecutionContext { public get extendedSelection(): any { return this._currentContext.extendedSelection; } + public get groupSlice(): any { + return this._currentContext.groupSlice; + } getCurrentPanelId(): string | null { return this.params.panel_id || this.currentPanel?.id || null; } @@ -538,6 +542,7 @@ async function executeOperatorAsGenerator( selected_labels: formatSelectedLabels(currentContext.selectedLabels), view: currentContext.view, view_name: currentContext.viewName, + group_slice: currentContext.groupSlice, }, "json-stream" ); @@ -700,6 +705,7 @@ export async function executeOperatorWithContext( selected_labels: formatSelectedLabels(currentContext.selectedLabels), view: currentContext.view, view_name: currentContext.viewName, + group_slice: currentContext.groupSlice, } ); result = serverResult.result; @@ -802,6 +808,7 @@ export async function resolveRemoteType( selected_labels: formatSelectedLabels(currentContext.selectedLabels), view: currentContext.view, view_name: currentContext.viewName, + group_slice: currentContext.groupSlice, } ); @@ -875,6 +882,7 @@ export async function resolveExecutionOptions( selected_labels: formatSelectedLabels(currentContext.selectedLabels), view: currentContext.view, view_name: currentContext.viewName, + group_slice: currentContext.groupSlice, } ); @@ -905,6 +913,7 @@ export async function fetchRemotePlacements(ctx: ExecutionContext) { selected_labels: formatSelectedLabels(currentContext.selectedLabels), current_sample: currentContext.currentSample, view_name: currentContext.viewName, + group_slice: currentContext.groupSlice, } ); if (result && result.error) { diff --git a/app/packages/operators/src/state.ts b/app/packages/operators/src/state.ts index f489395f1d..97e675c5d1 100644 --- a/app/packages/operators/src/state.ts +++ b/app/packages/operators/src/state.ts @@ -93,6 +93,7 @@ const globalContextSelector = selector({ const selectedLabels = get(fos.selectedLabels); const viewName = get(fos.viewName); const extendedSelection = get(fos.extendedSelection); + const groupSlice = get(fos.groupSlice); return { datasetName, @@ -103,6 +104,7 @@ const globalContextSelector = selector({ selectedLabels, viewName, extendedSelection, + groupSlice, }; }, }); @@ -142,6 +144,7 @@ const useExecutionContext = (operatorName, hooks = {}) => { selectedLabels, viewName, extendedSelection, + groupSlice, } = curCtx; const [analyticsInfo] = useAnalyticsInfo(); const ctx = useMemo(() => { @@ -158,6 +161,7 @@ const useExecutionContext = (operatorName, hooks = {}) => { viewName, extendedSelection, analyticsInfo, + groupSlice, }, hooks ); @@ -172,6 +176,7 @@ const useExecutionContext = (operatorName, hooks = {}) => { hooks, viewName, currentSample, + groupSlice, ]); return ctx; diff --git a/app/packages/operators/src/useCustomPanelHooks.ts b/app/packages/operators/src/useCustomPanelHooks.ts index ef0fc8088b..afbaf851dd 100644 --- a/app/packages/operators/src/useCustomPanelHooks.ts +++ b/app/packages/operators/src/useCustomPanelHooks.ts @@ -25,6 +25,7 @@ export interface CustomPanelProps { onChangeSelected?: string; onChangeSelectedLabels?: string; onChangeExtendedSelection?: string; + onChangeGroupSlice?: string; dimensions: DimensionsType | null; panelName?: string; panelLabel?: string; @@ -129,6 +130,12 @@ export function useCustomPanelHooks(props: CustomPanelProps): CustomPanelHooks { ctx.selectedLabels, props.onChangeSelectedLabels ); + useCtxChangePanelEvent( + isLoaded, + panelId, + ctx.groupSlice, + props.onChangeGroupSlice + ); useEffect(() => { onLoad(); diff --git a/docs/source/plugins/developing_plugins.rst b/docs/source/plugins/developing_plugins.rst index 3032689bdc..0bec46198d 100644 --- a/docs/source/plugins/developing_plugins.rst +++ b/docs/source/plugins/developing_plugins.rst @@ -2047,6 +2047,19 @@ subsequent sections. } ctx.panel.set_state("event", "on_change_extended_selection") ctx.panel.set_data("event_data", event) + + def on_change_group_slice(self, ctx): + """Implement this method to set panel state/data when the current + group slice changes. + + The current group slice will be available via ``ctx.group_slice``. + """ + event = { + "data": ctx.group_slice, + "description": "the current group slice", + } + ctx.panel.set_state("event", "on_change_group_slice") + ctx.panel.set_data("event_data", event) ####################################################################### # Custom events diff --git a/fiftyone/operators/executor.py b/fiftyone/operators/executor.py index 151343cd42..57df1b4b88 100644 --- a/fiftyone/operators/executor.py +++ b/fiftyone/operators/executor.py @@ -697,6 +697,11 @@ def ops(self): """ return self._ops + @property + def group_slice(self): + """The group slice of the view.""" + return self.request_params.get("group_slice", None) + def prompt( self, operator_uri, diff --git a/fiftyone/operators/operations.py b/fiftyone/operators/operations.py index 01d46dcc22..2af0e1a4fe 100644 --- a/fiftyone/operators/operations.py +++ b/fiftyone/operators/operations.py @@ -316,6 +316,7 @@ def register_panel( on_change_selected=None, on_change_selected_labels=None, on_change_extended_selection=None, + on_change_group_slice=None, allow_duplicates=False, ): """Registers a panel with the given name and lifecycle callbacks. @@ -353,6 +354,7 @@ def register_panel( current selected labels changes on_change_extended_selection (None): an operator to invoke when the current extended selection changes + on_change_group_slice (None): an operator to invoke when the group slice changes allow_duplicates (False): whether to allow multiple instances of the panel to the opened """ @@ -375,6 +377,7 @@ def register_panel( "on_change_selected": on_change_selected, "on_change_selected_labels": on_change_selected_labels, "on_change_extended_selection": on_change_extended_selection, + "on_change_group_slice": on_change_group_slice, "allow_duplicates": allow_duplicates, } return self._ctx.trigger("register_panel", params=params) diff --git a/fiftyone/operators/panel.py b/fiftyone/operators/panel.py index c2a032ec51..b9d897c2fd 100644 --- a/fiftyone/operators/panel.py +++ b/fiftyone/operators/panel.py @@ -125,6 +125,7 @@ def on_startup(self, ctx): "on_change_selected", "on_change_selected_labels", "on_change_extended_selection", + "on_change_group_slice", ] for method in methods + ctx_change_events: if hasattr(self, method) and callable(getattr(self, method)): From 60de3644cfdcb49324b238245f82b320223c30ee Mon Sep 17 00:00:00 2001 From: imanjra Date: Thu, 26 Sep 2024 10:21:23 -0400 Subject: [PATCH 2/4] group slice operator tweaks --- fiftyone/operators/builtin.py | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/fiftyone/operators/builtin.py b/fiftyone/operators/builtin.py index d879405b5b..1005cc44be 100644 --- a/fiftyone/operators/builtin.py +++ b/fiftyone/operators/builtin.py @@ -1519,9 +1519,8 @@ def execute(self, ctx): ctx.dataset.rename_group_slice(name, new_name) - # @todo ideally we would only set this if the group slice we renamed is - # currently loaded in the App - ctx.ops.set_group_slice(ctx.dataset.default_group_slice) + if ctx.group_slice == name: + ctx.ops.set_group_slice(ctx.dataset.default_group_slice) ctx.ops.reload_dataset() @@ -1567,13 +1566,9 @@ def resolve_input(self, ctx): def execute(self, ctx): name = ctx.params["name"] - ctx.dataset.delete_group_slice(name) - - # @todo ideally we would only set this if the group slice we deleted is - # currently loaded in the App - ctx.ops.set_group_slice(ctx.dataset.default_group_slice) - + if ctx.group_slice == name: + ctx.ops.set_group_slice(ctx.dataset.default_group_slice) ctx.ops.reload_dataset() From 808260c40b8a88df660a84278fba56a9cba59a8e Mon Sep 17 00:00:00 2001 From: brimoor Date: Thu, 26 Sep 2024 16:45:31 -0400 Subject: [PATCH 3/4] more group_slice updates --- docs/source/plugins/developing_plugins.rst | 1 + fiftyone/operators/builtin.py | 5 +++-- fiftyone/operators/executor.py | 13 ++++++++++++- fiftyone/operators/operations.py | 3 ++- 4 files changed, 18 insertions(+), 4 deletions(-) diff --git a/docs/source/plugins/developing_plugins.rst b/docs/source/plugins/developing_plugins.rst index 0bec46198d..6ac02fc7f9 100644 --- a/docs/source/plugins/developing_plugins.rst +++ b/docs/source/plugins/developing_plugins.rst @@ -979,6 +979,7 @@ contains the following properties: - `ctx.selected_labels` - the list of currently selected labels in the App, if any - `ctx.extended_selection` - the extended selection of the view, if any +- `ctx.group_slice` - the active group slice in the App, if any - `ctx.user_id` - the ID of the user that invoked the operator, if known - `ctx.panel_id` - the ID of the panel that invoked the operator, if any - `ctx.panel` - a :class:`PanelRef ` diff --git a/fiftyone/operators/builtin.py b/fiftyone/operators/builtin.py index 1005cc44be..bb07d5b3d1 100644 --- a/fiftyone/operators/builtin.py +++ b/fiftyone/operators/builtin.py @@ -1518,9 +1518,8 @@ def execute(self, ctx): new_name = ctx.params["new_name"] ctx.dataset.rename_group_slice(name, new_name) - if ctx.group_slice == name: - ctx.ops.set_group_slice(ctx.dataset.default_group_slice) + ctx.ops.set_group_slice(new_name) ctx.ops.reload_dataset() @@ -1566,9 +1565,11 @@ def resolve_input(self, ctx): def execute(self, ctx): name = ctx.params["name"] + ctx.dataset.delete_group_slice(name) if ctx.group_slice == name: ctx.ops.set_group_slice(ctx.dataset.default_group_slice) + ctx.ops.reload_dataset() diff --git a/fiftyone/operators/executor.py b/fiftyone/operators/executor.py index 57df1b4b88..8b5da0d5ea 100644 --- a/fiftyone/operators/executor.py +++ b/fiftyone/operators/executor.py @@ -17,6 +17,7 @@ import fiftyone as fo import fiftyone.core.dataset as fod +import fiftyone.core.media as fom import fiftyone.core.odm.utils as focu import fiftyone.core.utils as fou import fiftyone.core.view as fov @@ -507,11 +508,13 @@ def dataset(self): """The :class:`fiftyone.core.dataset.Dataset` being operated on.""" if self._dataset is not None: return self._dataset + # Since dataset may have been renamed, always resolve the dataset by # id if it is available uid = self.request_params.get("dataset_id", None) if uid: self._dataset = focu.load_dataset(id=uid) + # Set the dataset_name using the dataset object in case the dataset # has been renamed or changed since the context was created self.request_params["dataset_name"] = self._dataset.name @@ -519,10 +522,18 @@ def dataset(self): uid = self.request_params.get("dataset_name", None) if uid: self._dataset = focu.load_dataset(name=uid) + # TODO: refactor so that this additional reload post-load is not # required if self._dataset is not None: self._dataset.reload() + + if ( + self.group_slice is not None + and self._dataset.media_type == fom.GROUP + ): + self._dataset.group_slice = self.group_slice + return self._dataset @property @@ -699,7 +710,7 @@ def ops(self): @property def group_slice(self): - """The group slice of the view.""" + """The current group slice of the view (if any).""" return self.request_params.get("group_slice", None) def prompt( diff --git a/fiftyone/operators/operations.py b/fiftyone/operators/operations.py index 2af0e1a4fe..933f6f3d55 100644 --- a/fiftyone/operators/operations.py +++ b/fiftyone/operators/operations.py @@ -354,7 +354,8 @@ def register_panel( current selected labels changes on_change_extended_selection (None): an operator to invoke when the current extended selection changes - on_change_group_slice (None): an operator to invoke when the group slice changes + on_change_group_slice (None): an operator to invoke when the group + slice changes allow_duplicates (False): whether to allow multiple instances of the panel to the opened """ From e38dfd0a4552a579161b6be01a015f35d0d9fe51 Mon Sep 17 00:00:00 2001 From: brimoor Date: Thu, 26 Sep 2024 16:50:31 -0400 Subject: [PATCH 4/4] intelligent default --- fiftyone/operators/builtin.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fiftyone/operators/builtin.py b/fiftyone/operators/builtin.py index bb07d5b3d1..d754ee5019 100644 --- a/fiftyone/operators/builtin.py +++ b/fiftyone/operators/builtin.py @@ -1487,7 +1487,7 @@ def resolve_input(self, ctx): inputs.enum( "name", slice_selector.values(), - default=None, + default=ctx.group_slice, required=True, label="Group slice", description="The group slice to rename", @@ -1552,7 +1552,7 @@ def resolve_input(self, ctx): inputs.enum( "name", slice_selector.values(), - default=None, + default=ctx.group_slice, required=True, label="Group slice", description="The group slice to delete",