From e2a23ba10b92080007d2f7243e3201f5cdca8240 Mon Sep 17 00:00:00 2001 From: Benjamin Kane Date: Fri, 22 Nov 2024 20:04:58 -0500 Subject: [PATCH 1/6] fix delete and recreate dataset for server --- fiftyone/core/singletons.py | 5 ++++- fiftyone/server/query.py | 6 +----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/fiftyone/core/singletons.py b/fiftyone/core/singletons.py index 9068dfa44e..ec2976a5c2 100644 --- a/fiftyone/core/singletons.py +++ b/fiftyone/core/singletons.py @@ -5,6 +5,7 @@ | `voxel51.com `_ | """ + from collections import defaultdict import weakref @@ -37,7 +38,9 @@ def __call__(cls, name=None, _create=True, *args, **kwargs): name = instance.name # `__init__` may have changed `name` else: try: - instance._update_last_loaded_at() + instance._update_last_loaded_at( + force=kwargs.get("force_load", False) + ) except ValueError: instance._deleted = True return cls.__call__( diff --git a/fiftyone/server/query.py b/fiftyone/server/query.py index ee5c11894a..ef96f24edf 100644 --- a/fiftyone/server/query.py +++ b/fiftyone/server/query.py @@ -585,11 +585,7 @@ def run(): if not fod.dataset_exists(dataset_name): return None - dataset = fod.load_dataset(dataset_name) - - if update_last_loaded_at: - dataset._update_last_loaded_at(force=True) - + dataset = fo.Dataset(dataset_name, _create=False, force_load=True) dataset.reload() view_name = None try: From 4d1724192dd0ac2f13b4f7d5e19e80b4620c880a Mon Sep 17 00:00:00 2001 From: Benjamin Kane Date: Mon, 25 Nov 2024 10:17:32 -0500 Subject: [PATCH 2/6] _force_load --- fiftyone/core/singletons.py | 2 +- fiftyone/server/query.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/fiftyone/core/singletons.py b/fiftyone/core/singletons.py index ec2976a5c2..0940152d5f 100644 --- a/fiftyone/core/singletons.py +++ b/fiftyone/core/singletons.py @@ -39,7 +39,7 @@ def __call__(cls, name=None, _create=True, *args, **kwargs): else: try: instance._update_last_loaded_at( - force=kwargs.get("force_load", False) + force=kwargs.get("_force_load", False) ) except ValueError: instance._deleted = True diff --git a/fiftyone/server/query.py b/fiftyone/server/query.py index ef96f24edf..7ae0fcf07d 100644 --- a/fiftyone/server/query.py +++ b/fiftyone/server/query.py @@ -585,7 +585,7 @@ def run(): if not fod.dataset_exists(dataset_name): return None - dataset = fo.Dataset(dataset_name, _create=False, force_load=True) + dataset = fo.Dataset(dataset_name, _create=False, _force_load=True) dataset.reload() view_name = None try: From 4d554d6458249af582dceb33742253c87fd27a2c Mon Sep 17 00:00:00 2001 From: Benjamin Kane Date: Mon, 25 Nov 2024 12:12:33 -0500 Subject: [PATCH 3/6] merge groups --- app/packages/state/src/recoil/sidebar.ts | 45 ++++++++++++++++++++++++ fiftyone/core/state.py | 4 ++- 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/app/packages/state/src/recoil/sidebar.ts b/app/packages/state/src/recoil/sidebar.ts index d68c42865e..c974b262af 100644 --- a/app/packages/state/src/recoil/sidebar.ts +++ b/app/packages/state/src/recoil/sidebar.ts @@ -196,6 +196,47 @@ const DEFAULT_VIDEO_GROUPS: State.SidebarGroup[] = [ const NONE = [null, undefined]; +const insertFromNeighbor = (sink: string[], source: string[], key: string) => { + if (sink.includes(key)) { + return; + } + const sourceIndex = source.indexOf(key); + const neighbor = source[sourceIndex - 1]; + const neighborIndex = sink.indexOf(neighbor); + + !neighbor ? sink.push(key) : sink.splice(neighborIndex + 1, 0, key); +}; + +const mergeGroups = ( + sink: State.SidebarGroup[], + source: State.SidebarGroup[] +) => { + const mapping = Object.fromEntries(sink.map((g) => [g.name, g])); + const configMapping = Object.fromEntries(source.map((g) => [g.name, g])); + const sinkKeys = sink.map(({ name }) => name); + const sourceKeys = source.map(({ name }) => name); + for (const key of sourceKeys) { + insertFromNeighbor(sinkKeys, sourceKeys, key); + } + + const resolved = sink.map((g) => mapping[g] ?? configMapping[g]); + for (const g in sink) { + const i = source.indexOf(g); + + if (i < 0) { + continue; + } + + const gPaths = source[i].paths; + + for (const p in gPaths) { + insertFromNeighbor(mapping[g].paths, gPaths, p); + } + } + + return resolved; +}; + export const resolveGroups = ( sampleFields: StrictField[], frameFields: StrictField[], @@ -210,6 +251,10 @@ export const resolveGroups = ( ? DEFAULT_VIDEO_GROUPS : DEFAULT_IMAGE_GROUPS; + if (currentGroups.length && configGroups.length) { + groups = mergeGroups(groups, configGroups); + } + const expanded = configGroups.reduce((map, { name, expanded }) => { map[name] = expanded; return map; diff --git a/fiftyone/core/state.py b/fiftyone/core/state.py index fe403f72d0..e924d3d162 100644 --- a/fiftyone/core/state.py +++ b/fiftyone/core/state.py @@ -97,7 +97,9 @@ def serialize(self, reflective=True): if self.dataset is not None: d["dataset"] = self.dataset.name - collection = self.dataset + collection = fo.Dataset( + self.dataset.name, _create=False, _force_load=True + ) if self.view is not None: collection = self.view From d058feb05d6ce960d2fd27dca538159031941e76 Mon Sep 17 00:00:00 2001 From: Benjamin Kane Date: Mon, 25 Nov 2024 13:23:44 -0500 Subject: [PATCH 4/6] rudimentary merges, add test --- .../recoil/{sidebar.ts => sidebar/index.ts} | 65 +++------------- .../src/recoil/sidebar/sidebar-utils.test.ts | 33 ++++++++ .../state/src/recoil/sidebar/sidebar-utils.ts | 77 +++++++++++++++++++ 3 files changed, 122 insertions(+), 53 deletions(-) rename app/packages/state/src/recoil/{sidebar.ts => sidebar/index.ts} (94%) create mode 100644 app/packages/state/src/recoil/sidebar/sidebar-utils.test.ts create mode 100644 app/packages/state/src/recoil/sidebar/sidebar-utils.ts diff --git a/app/packages/state/src/recoil/sidebar.ts b/app/packages/state/src/recoil/sidebar/index.ts similarity index 94% rename from app/packages/state/src/recoil/sidebar.ts rename to app/packages/state/src/recoil/sidebar/index.ts index c974b262af..11ae2e0fb6 100644 --- a/app/packages/state/src/recoil/sidebar.ts +++ b/app/packages/state/src/recoil/sidebar/index.ts @@ -34,17 +34,17 @@ import { useRecoilStateLoadable, useRecoilValueLoadable, } from "recoil"; -import { collapseFields, getCurrentEnvironment } from "../utils"; -import * as atoms from "./atoms"; -import { getBrowserStorageEffectForKey } from "./customEffects"; +import { collapseFields, getCurrentEnvironment } from "../../utils"; +import * as atoms from "../atoms"; +import { getBrowserStorageEffectForKey } from "../customEffects"; import { active3dSlices, active3dSlicesToSampleMap, activeModalSidebarSample, pinned3DSampleSlice, -} from "./groups"; -import { isLargeVideo } from "./options"; -import { cumulativeValues, values } from "./pathData"; +} from "../groups"; +import { isLargeVideo } from "../options"; +import { cumulativeValues, values } from "../pathData"; import { buildSchema, field, @@ -53,23 +53,23 @@ import { filterPaths, isOfDocumentFieldList, pathIsShown, -} from "./schema"; -import { isFieldVisibilityActive } from "./schemaSettings.atoms"; +} from "../schema"; +import { isFieldVisibilityActive } from "../schemaSettings.atoms"; import { datasetName, disableFrameFiltering, isVideoDataset, stateSubscription, -} from "./selectors"; -import { State } from "./types"; +} from "../selectors"; +import { State } from "../types"; import { fieldsMatcher, groupFilter, labelsMatcher, primitivesMatcher, unsupportedMatcher, -} from "./utils"; -import * as viewAtoms from "./view"; +} from "../utils"; +import * as viewAtoms from "../view"; export enum EntryKind { EMPTY = "EMPTY", @@ -196,47 +196,6 @@ const DEFAULT_VIDEO_GROUPS: State.SidebarGroup[] = [ const NONE = [null, undefined]; -const insertFromNeighbor = (sink: string[], source: string[], key: string) => { - if (sink.includes(key)) { - return; - } - const sourceIndex = source.indexOf(key); - const neighbor = source[sourceIndex - 1]; - const neighborIndex = sink.indexOf(neighbor); - - !neighbor ? sink.push(key) : sink.splice(neighborIndex + 1, 0, key); -}; - -const mergeGroups = ( - sink: State.SidebarGroup[], - source: State.SidebarGroup[] -) => { - const mapping = Object.fromEntries(sink.map((g) => [g.name, g])); - const configMapping = Object.fromEntries(source.map((g) => [g.name, g])); - const sinkKeys = sink.map(({ name }) => name); - const sourceKeys = source.map(({ name }) => name); - for (const key of sourceKeys) { - insertFromNeighbor(sinkKeys, sourceKeys, key); - } - - const resolved = sink.map((g) => mapping[g] ?? configMapping[g]); - for (const g in sink) { - const i = source.indexOf(g); - - if (i < 0) { - continue; - } - - const gPaths = source[i].paths; - - for (const p in gPaths) { - insertFromNeighbor(mapping[g].paths, gPaths, p); - } - } - - return resolved; -}; - export const resolveGroups = ( sampleFields: StrictField[], frameFields: StrictField[], diff --git a/app/packages/state/src/recoil/sidebar/sidebar-utils.test.ts b/app/packages/state/src/recoil/sidebar/sidebar-utils.test.ts new file mode 100644 index 0000000000..d258a6c553 --- /dev/null +++ b/app/packages/state/src/recoil/sidebar/sidebar-utils.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, it, vi } from "vitest"; + +vi.mock("recoil"); +vi.mock("recoil-relay"); + +import { mergeGroups } from "./sidebar-utils"; + +describe("test sidebar groups resolution", () => { + it("merges current and config groups", () => { + expect( + mergeGroups( + [ + { name: "one", paths: ["one.one", "one.three"] }, + { name: "three", paths: [] }, + ], + + [ + { name: "zero", paths: [] }, + { + name: "one", + paths: ["one.zero", "one.one", "one.two"], + }, + { name: "two", paths: [] }, + ] + ) + ).toStrictEqual([ + { name: "zero", paths: [] }, + { name: "one", paths: ["one.zero", "one.one", "one.two", "one.three"] }, + { name: "two", paths: [] }, + { name: "three", paths: [] }, + ]); + }); +}); diff --git a/app/packages/state/src/recoil/sidebar/sidebar-utils.ts b/app/packages/state/src/recoil/sidebar/sidebar-utils.ts new file mode 100644 index 0000000000..98b335faeb --- /dev/null +++ b/app/packages/state/src/recoil/sidebar/sidebar-utils.ts @@ -0,0 +1,77 @@ +import type { State } from "../types"; + +const hasNeighbor = (sink: string[], source: string[], key: string) => { + const index = source.indexOf(key); + const before = source[index - 1]; + const after = source[index + 1]; + + return sink.includes(before) || sink.includes(after); +}; + +const insertFromNeighbor = (sink: string[], source: string[], key: string) => { + if (sink.includes(key)) { + return; + } + + const sourceIndex = source.indexOf(key); + const before = source[sourceIndex - 1]; + const beforeIndex = sink.indexOf(before); + + if (beforeIndex >= 0) { + sink.splice(beforeIndex + 1, 0, key); + return; + } + + const after = source[sourceIndex + 1]; + const afterIndex = sink.indexOf(after); + + if (afterIndex >= 0) { + sink.splice(afterIndex, 0, key); + return; + } + + sink.push(key); + return; +}; + +const merge = (sink: string[], source: string[]) => { + const missing = new Set(source.filter((key) => !sink.includes(key))); + + while (missing.size) { + const force = ![...missing].some((name) => hasNeighbor(sink, source, name)); + for (const name of missing) { + if (!force && !hasNeighbor(sink, source, name)) { + continue; + } + insertFromNeighbor(sink, source, name); + missing.delete(name); + } + } +}; + +export const mergeGroups = ( + sink: State.SidebarGroup[], + source: State.SidebarGroup[] +) => { + const mapping = Object.fromEntries(sink.map((g) => [g.name, g])); + const configMapping = Object.fromEntries(source.map((g) => [g.name, g])); + const sinkKeys = sink.map(({ name }) => name); + const sourceKeys = source.map(({ name }) => name); + + merge(sinkKeys, sourceKeys); + + for (const key of sinkKeys) { + mapping[key] = mapping[key] ?? configMapping[key]; + } + const resolved = sinkKeys.map((g) => mapping[g] ?? configMapping[g]); + for (const { name } of resolved) { + const i = sourceKeys.indexOf(name); + if (i < 0) { + continue; + } + + merge(mapping[name].paths || [], source[i].paths); + } + + return resolved; +}; From a0901ceca9f3f45d085da557323b0882ed3bbf21 Mon Sep 17 00:00:00 2001 From: Benjamin Kane Date: Mon, 25 Nov 2024 14:26:02 -0500 Subject: [PATCH 5/6] cleanup --- app/packages/state/src/recoil/sidebar/index.ts | 1 + .../state/src/recoil/sidebar/sidebar-utils.test.ts | 6 +++++- .../state/src/recoil/sidebar/sidebar-utils.ts | 13 ++++++++++--- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/app/packages/state/src/recoil/sidebar/index.ts b/app/packages/state/src/recoil/sidebar/index.ts index 11ae2e0fb6..7c4aa45cfb 100644 --- a/app/packages/state/src/recoil/sidebar/index.ts +++ b/app/packages/state/src/recoil/sidebar/index.ts @@ -70,6 +70,7 @@ import { unsupportedMatcher, } from "../utils"; import * as viewAtoms from "../view"; +import { mergeGroups } from "./sidebar-utils"; export enum EntryKind { EMPTY = "EMPTY", diff --git a/app/packages/state/src/recoil/sidebar/sidebar-utils.test.ts b/app/packages/state/src/recoil/sidebar/sidebar-utils.test.ts index d258a6c553..5716e01cef 100644 --- a/app/packages/state/src/recoil/sidebar/sidebar-utils.test.ts +++ b/app/packages/state/src/recoil/sidebar/sidebar-utils.test.ts @@ -3,9 +3,13 @@ import { describe, expect, it, vi } from "vitest"; vi.mock("recoil"); vi.mock("recoil-relay"); -import { mergeGroups } from "./sidebar-utils"; +import { merge, mergeGroups } from "./sidebar-utils"; describe("test sidebar groups resolution", () => { + it("test list merge", () => { + expect(merge([], ["one", "two"])).toStrictEqual(["one", "two"]); + }); + it("merges current and config groups", () => { expect( mergeGroups( diff --git a/app/packages/state/src/recoil/sidebar/sidebar-utils.ts b/app/packages/state/src/recoil/sidebar/sidebar-utils.ts index 98b335faeb..d9292c5729 100644 --- a/app/packages/state/src/recoil/sidebar/sidebar-utils.ts +++ b/app/packages/state/src/recoil/sidebar/sidebar-utils.ts @@ -34,19 +34,25 @@ const insertFromNeighbor = (sink: string[], source: string[], key: string) => { return; }; -const merge = (sink: string[], source: string[]) => { +export const merge = (sink: string[], source: string[]) => { const missing = new Set(source.filter((key) => !sink.includes(key))); while (missing.size) { - const force = ![...missing].some((name) => hasNeighbor(sink, source, name)); + const force = [...missing].every( + (name) => !hasNeighbor(sink, source, name) + ); for (const name of missing) { if (!force && !hasNeighbor(sink, source, name)) { continue; } + insertFromNeighbor(sink, source, name); + missing.delete(name); } } + + return sink; }; export const mergeGroups = ( @@ -70,7 +76,8 @@ export const mergeGroups = ( continue; } - merge(mapping[name].paths || [], source[i].paths); + mapping[name].paths = mapping[name].paths ?? []; + merge(mapping[name].paths, source[i].paths); } return resolved; From efed29a7bae4e48e7db24602d1676490b1c0989b Mon Sep 17 00:00:00 2001 From: Benjamin Kane Date: Mon, 25 Nov 2024 14:56:01 -0500 Subject: [PATCH 6/6] handle readonly --- .../state/src/recoil/sidebar/sidebar-utils.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/app/packages/state/src/recoil/sidebar/sidebar-utils.ts b/app/packages/state/src/recoil/sidebar/sidebar-utils.ts index d9292c5729..9459110749 100644 --- a/app/packages/state/src/recoil/sidebar/sidebar-utils.ts +++ b/app/packages/state/src/recoil/sidebar/sidebar-utils.ts @@ -59,8 +59,12 @@ export const mergeGroups = ( sink: State.SidebarGroup[], source: State.SidebarGroup[] ) => { - const mapping = Object.fromEntries(sink.map((g) => [g.name, g])); - const configMapping = Object.fromEntries(source.map((g) => [g.name, g])); + // make copies, assume readonly + const mapping = Object.fromEntries(sink.map((g) => [g.name, { ...g }])); + const configMapping = Object.fromEntries( + source.map((g) => [g.name, { ...g }]) + ); + const sinkKeys = sink.map(({ name }) => name); const sourceKeys = source.map(({ name }) => name); @@ -76,8 +80,9 @@ export const mergeGroups = ( continue; } - mapping[name].paths = mapping[name].paths ?? []; - merge(mapping[name].paths, source[i].paths); + // make copies, assume readonly + mapping[name].paths = [...(mapping[name].paths ?? [])]; + merge(mapping[name].paths, [...source[i].paths]); } return resolved;