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

better group slice support for operators #4850

Merged
merged 5 commits into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions app/packages/operators/src/CustomPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
setPanelCloseEffect(() => {
clearUseKeyStores(panelId);
});
}, []);

Check warning on line 37 in app/packages/operators/src/CustomPanel.tsx

View workflow job for this annotation

GitHub Actions / lint / eslint

React Hook useEffect has missing dependencies: 'panelId' and 'setPanelCloseEffect'. Either include them or remove the dependency array

useEffect(() => {
setLoading(count > 0);
Expand Down Expand Up @@ -98,7 +98,7 @@

useEffect(() => {
dimensions?.refresh();
}, []);

Check warning on line 101 in app/packages/operators/src/CustomPanel.tsx

View workflow job for this annotation

GitHub Actions / lint / eslint

React Hook useEffect has a missing dependency: 'dimensions'. Either include it or remove the dependency array

return children;
}
Expand All @@ -114,6 +114,7 @@
on_change_selected,
on_change_selected_labels,
on_change_extended_selection,
on_change_group_slice,
panel_name,
panel_label,
}) {
Expand All @@ -130,6 +131,7 @@
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}
Expand Down
3 changes: 3 additions & 0 deletions app/packages/operators/src/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand All @@ -47,6 +48,7 @@ function useOperatorThrottledContextSetter() {
selectedLabels,
currentSample,
viewName,
groupSlice,
});
}, [
setThrottledContext,
Expand All @@ -58,6 +60,7 @@ function useOperatorThrottledContextSetter() {
selectedLabels,
currentSample,
viewName,
groupSlice,
]);
}

Expand Down
9 changes: 9 additions & 0 deletions app/packages/operators/src/operators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ export type RawContext = {
selection: string[] | null;
scope: string;
};
groupSlice: string;
};

export class ExecutionContext {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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"
);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -802,6 +808,7 @@ export async function resolveRemoteType(
selected_labels: formatSelectedLabels(currentContext.selectedLabels),
view: currentContext.view,
view_name: currentContext.viewName,
group_slice: currentContext.groupSlice,
}
);

Expand Down Expand Up @@ -875,6 +882,7 @@ export async function resolveExecutionOptions(
selected_labels: formatSelectedLabels(currentContext.selectedLabels),
view: currentContext.view,
view_name: currentContext.viewName,
group_slice: currentContext.groupSlice,
}
);

Expand Down Expand Up @@ -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) {
Expand Down
5 changes: 5 additions & 0 deletions app/packages/operators/src/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -103,6 +104,7 @@ const globalContextSelector = selector({
selectedLabels,
viewName,
extendedSelection,
groupSlice,
};
},
});
Expand Down Expand Up @@ -142,6 +144,7 @@ const useExecutionContext = (operatorName, hooks = {}) => {
selectedLabels,
viewName,
extendedSelection,
groupSlice,
} = curCtx;
const [analyticsInfo] = useAnalyticsInfo();
const ctx = useMemo(() => {
Expand All @@ -158,6 +161,7 @@ const useExecutionContext = (operatorName, hooks = {}) => {
viewName,
extendedSelection,
analyticsInfo,
groupSlice,
},
hooks
);
Expand All @@ -172,6 +176,7 @@ const useExecutionContext = (operatorName, hooks = {}) => {
hooks,
viewName,
currentSample,
groupSlice,
]);

return ctx;
Expand Down
7 changes: 7 additions & 0 deletions app/packages/operators/src/useCustomPanelHooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export interface CustomPanelProps {
onChangeSelected?: string;
onChangeSelectedLabels?: string;
onChangeExtendedSelection?: string;
onChangeGroupSlice?: string;
dimensions: DimensionsType | null;
panelName?: string;
panelLabel?: string;
Expand Down Expand Up @@ -129,6 +130,12 @@ export function useCustomPanelHooks(props: CustomPanelProps): CustomPanelHooks {
ctx.selectedLabels,
props.onChangeSelectedLabels
);
useCtxChangePanelEvent(
isLoaded,
panelId,
ctx.groupSlice,
props.onChangeGroupSlice
);

useEffect(() => {
onLoad();
Expand Down
14 changes: 14 additions & 0 deletions docs/source/plugins/developing_plugins.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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 <fiftyone.operators.panel.PanelRef>`
Expand Down Expand Up @@ -2047,6 +2048,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
Expand Down
16 changes: 6 additions & 10 deletions fiftyone/operators/builtin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -1518,10 +1518,8 @@ def execute(self, ctx):
new_name = ctx.params["new_name"]

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(new_name)

ctx.ops.reload_dataset()

Expand Down Expand Up @@ -1554,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",
Expand All @@ -1569,10 +1567,8 @@ 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()

Expand Down
16 changes: 16 additions & 0 deletions fiftyone/operators/executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -507,22 +508,32 @@ 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
else:
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
Expand Down Expand Up @@ -697,6 +708,11 @@ def ops(self):
"""
return self._ops

@property
def group_slice(self):
"""The current group slice of the view (if any)."""
return self.request_params.get("group_slice", None)

def prompt(
self,
operator_uri,
Expand Down
4 changes: 4 additions & 0 deletions fiftyone/operators/operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -353,6 +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
allow_duplicates (False): whether to allow multiple instances of
the panel to the opened
"""
Expand All @@ -375,6 +378,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)
Expand Down
1 change: 1 addition & 0 deletions fiftyone/operators/panel.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ def on_startup(self, ctx):
"on_change_selected",
"on_change_selected_labels",
"on_change_extended_selection",
"on_change_group_slice",
brimoor marked this conversation as resolved.
Show resolved Hide resolved
]
for method in methods + ctx_change_events:
if hasattr(self, method) and callable(getattr(self, method)):
Expand Down
Loading