From 1fdd8708647a0ffdf90642c1b9045c8bc398125f Mon Sep 17 00:00:00 2001 From: Benjamin Kane Date: Fri, 30 Aug 2024 12:34:04 -0400 Subject: [PATCH 1/6] Fix sample selection with map panel (#4739) * clear records on page refresh * refactor as independent hook, add hook test * rm useRef * fixing selections with partial index records * add range tests * relay cleanup tweak * typos --- .../core/src/components/Grid/Grid.tsx | 30 +++--- .../src/components/Grid/useRecords.test.ts | 23 +++++ .../core/src/components/Grid/useRecords.ts | 10 ++ .../core/src/components/Grid/useSelect.ts | 2 +- .../components/Grid/useSelectSample.test.ts | 29 ++++++ .../src/components/Grid/useSelectSample.ts | 94 ++++++++++++------- .../src/components/Grid/useSpotlightPager.ts | 34 ++++--- 7 files changed, 159 insertions(+), 63 deletions(-) create mode 100644 app/packages/core/src/components/Grid/useRecords.test.ts create mode 100644 app/packages/core/src/components/Grid/useRecords.ts create mode 100644 app/packages/core/src/components/Grid/useSelectSample.test.ts diff --git a/app/packages/core/src/components/Grid/Grid.tsx b/app/packages/core/src/components/Grid/Grid.tsx index e1d848b9e8c..1ad019455e9 100644 --- a/app/packages/core/src/components/Grid/Grid.tsx +++ b/app/packages/core/src/components/Grid/Grid.tsx @@ -1,9 +1,9 @@ import styles from "./Grid.module.css"; -import type { Lookers } from "@fiftyone/state"; - import { freeVideos } from "@fiftyone/looker"; -import Spotlight, { type ID } from "@fiftyone/spotlight"; +import type { ID } from "@fiftyone/spotlight"; +import Spotlight from "@fiftyone/spotlight"; +import type { Lookers } from "@fiftyone/state"; import * as fos from "@fiftyone/state"; import React, { useEffect, @@ -17,6 +17,7 @@ import { v4 as uuid } from "uuid"; import { gridCrop, gridSpacing, pageParameters } from "./recoil"; import useAt from "./useAt"; import useEscape from "./useEscape"; +import useRecords from "./useRecords"; import useRefreshers from "./useRefreshers"; import useSelect from "./useSelect"; import useSelectSample from "./useSelectSample"; @@ -26,19 +27,22 @@ import useThreshold from "./useThreshold"; function Grid() { const id = useMemo(() => uuid(), []); const lookerStore = useMemo(() => new WeakMap(), []); - const selectSample = useRef>(); - const [resizing, setResizing] = useState(false); - const spacing = useRecoilValue(gridSpacing); + const selectSample = useRef>(); const { pageReset, reset } = useRefreshers(); - const { get, set } = useAt(pageReset); + const [resizing, setResizing] = useState(false); const threshold = useThreshold(); - const { page, records, store } = useSpotlightPager({ - pageSelector: pageParameters, - zoomSelector: gridCrop, - }); + const records = useRecords(pageReset); + const { page, store } = useSpotlightPager( + { + pageSelector: pageParameters, + zoomSelector: gridCrop, + }, + records + ); + const { get, set } = useAt(pageReset); const lookerOptions = fos.useLookerOptions(false); const createLooker = fos.useCreateLooker(false, true, lookerOptions); @@ -71,7 +75,7 @@ function Grid() { const init = (l) => { l.addEventListener("selectthumbnail", ({ detail }: CustomEvent) => { - selectSample.current?.(records, detail); + selectSample.current?.(detail); }); lookerStore.set(id, l); l.attach(element, dimensions); @@ -97,7 +101,7 @@ function Grid() { store, threshold, ]); - selectSample.current = useSelectSample(); + selectSample.current = useSelectSample(records); useSelect(lookerOptions, lookerStore, spotlight); useLayoutEffect(() => { diff --git a/app/packages/core/src/components/Grid/useRecords.test.ts b/app/packages/core/src/components/Grid/useRecords.test.ts new file mode 100644 index 00000000000..28f49e7dff2 --- /dev/null +++ b/app/packages/core/src/components/Grid/useRecords.test.ts @@ -0,0 +1,23 @@ +import { act, renderHook } from "@testing-library/react-hooks"; +import { describe, expect, it } from "vitest"; +import useRecords from "./useRecords"; + +describe("useRecords", () => { + it("return new records when clear string changes", () => { + const { result, rerender } = renderHook( + (clear: string) => useRecords(clear), + { initialProps: "one" } + ); + expect(result.current.size).toBe(0); + + act(() => { + result.current.set("one", 1); + }); + + expect(result.current.size).toBe(1); + + rerender("two"); + + expect(result.current.size).toBe(0); + }); +}); diff --git a/app/packages/core/src/components/Grid/useRecords.ts b/app/packages/core/src/components/Grid/useRecords.ts new file mode 100644 index 00000000000..f25b5f12d61 --- /dev/null +++ b/app/packages/core/src/components/Grid/useRecords.ts @@ -0,0 +1,10 @@ +import { useMemo } from "react"; + +export type Records = Map; + +export default (clear: string) => { + return useMemo(() => { + clear; + return new Map(); + }, [clear]); +}; diff --git a/app/packages/core/src/components/Grid/useSelect.ts b/app/packages/core/src/components/Grid/useSelect.ts index 809f524f8d6..8973c436d90 100644 --- a/app/packages/core/src/components/Grid/useSelect.ts +++ b/app/packages/core/src/components/Grid/useSelect.ts @@ -1,5 +1,5 @@ import type Spotlight from "@fiftyone/spotlight"; -import { ID } from "@fiftyone/spotlight"; +import type { ID } from "@fiftyone/spotlight"; import * as fos from "@fiftyone/state"; import { useEffect } from "react"; import { useRecoilValue } from "recoil"; diff --git a/app/packages/core/src/components/Grid/useSelectSample.test.ts b/app/packages/core/src/components/Grid/useSelectSample.test.ts new file mode 100644 index 00000000000..99385e738e8 --- /dev/null +++ b/app/packages/core/src/components/Grid/useSelectSample.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, it } from "vitest"; +import { addRange, removeRange } from "./useSelectSample"; + +describe("range selection tests", () => { + it("adds a range, and includes selections without an index record", () => { + const result = addRange( + 2, + new Set(["0", "other"]), + new Map([ + ["0", 0], + ["1", 1], + ["2", 2], + ]) + ); + expect(result).toStrictEqual(new Set(["0", "1", "2", "other"])); + }); + + it("removes a range, and includes selections without an index record", () => { + const result = removeRange( + 1, + new Set(["0", "1", "other"]), + new Map([ + ["0", 0], + ["1", 1], + ]) + ); + expect(result).toStrictEqual(new Set(["other"])); + }); +}); diff --git a/app/packages/core/src/components/Grid/useSelectSample.ts b/app/packages/core/src/components/Grid/useSelectSample.ts index 82b2dd05094..5b5cb096fbe 100644 --- a/app/packages/core/src/components/Grid/useSelectSample.ts +++ b/app/packages/core/src/components/Grid/useSelectSample.ts @@ -1,13 +1,12 @@ -import { ID } from "@fiftyone/spotlight"; +import type { ID } from "@fiftyone/spotlight"; import type { Sample } from "@fiftyone/state"; - import { selectedSampleObjects, selectedSamples, useSetSelected, } from "@fiftyone/state"; -import { MutableRefObject } from "react"; import { useRecoilCallback } from "recoil"; +import type { Records } from "./useRecords"; export interface SelectThumbnailData { shiftKey: boolean; @@ -16,23 +15,19 @@ export interface SelectThumbnailData { symbol: ID; } -type Records = MutableRefObject>; - -const argFact = (compareFn) => (array) => - array.map((el, idx) => [el, idx]).reduce(compareFn)[1]; - -const argMin = argFact((max, el) => (el[0] < max[0] ? el : max)); - -const addRange = (index: number, items: string[], records: Records) => { +export const addRange = ( + index: number, + selected: Set, + records: Records +) => { + // filter selections without an index record + const items = [...selected].filter((i) => records.has(i)); const reverse = Object.fromEntries( - Array.from(records.current.entries()).map(([k, v]) => [v, k]) - ); - - const min = argMin( - items.map((id) => Math.abs(records.current.get(id) - index)) + Array.from(records.entries()).map(([k, v]) => [v, k]) ); + const min = argMin(items.map((id) => Math.abs(get(records, id) - index))); - const close = records.current.get(items[min]); + const close = get(records, items[min]); const [start, end] = index < close ? [index, close] : [close, index]; @@ -40,26 +35,42 @@ const addRange = (index: number, items: string[], records: Records) => { .fill(0) .map((_, i) => reverse[i + start]); - return new Set([...items, ...added]); + return new Set([...selected, ...added]); }; -const removeRange = ( +const argFact = (compareFn) => (array) => + array.map((el, idx) => [el, idx]).reduce(compareFn)[1]; + +const argMin = argFact((max, el) => (el[0] < max[0] ? el : max)); + +const get = (records: Records, id: string) => { + const index = records.get(id); + if (index !== undefined) { + return index; + } + + throw new Error(`record '${id}' not found`); +}; + +export const removeRange = ( index: number, selected: Set, records: Records ) => { + // filter selections without an index record + const items = new Set([...selected].filter((i) => records.has(i))); const reverse = Object.fromEntries( - Array.from(records.current.entries()).map(([k, v]) => [v, k]) + Array.from(records.entries()).map(([k, v]) => [v, k]) ); let before = index; - while (selected.has(reverse[before])) { + while (items.has(reverse[before])) { before--; } before += 1; let after = index; - while (selected.has(reverse[after])) { + while (items.has(reverse[after])) { after++; } after -= 1; @@ -73,31 +84,44 @@ const removeRange = ( ? [before, index] : [index, after]; - return new Set( - Array.from(selected).filter( - (s) => records.current.get(s) < start || records.current.get(s) > end + const next = new Set( + Array.from(items).filter( + (s) => get(records, s) < start || get(records, s) > end ) ); + + for (const id of selected) { + if (records.has(id)) continue; + // not in index records so it was not removed, add it back + next.add(id); + } + + return next; }; -export default () => { +export default (records: Records) => { const setSelected = useSetSelected(); return useRecoilCallback( ({ set, snapshot }) => - async ( - records: Records, - { shiftKey, id: sampleId, sample, symbol }: SelectThumbnailData - ) => { - let selected = new Set(await snapshot.getPromise(selectedSamples)); + async ({ + shiftKey, + id: sampleId, + sample, + symbol, + }: SelectThumbnailData) => { + const current = new Set(await snapshot.getPromise(selectedSamples)); + let selected = new Set(current); const selectedObjects = new Map( await snapshot.getPromise(selectedSampleObjects) ); - const items = Array.from(selected); - const index = records.current.get(symbol.description); + const index = get(records, symbol.description); if (shiftKey && !selected.has(sampleId)) { - selected = addRange(index, items, records); + selected = new Set([ + ...selected, + ...addRange(index, selected, records), + ]); } else if (shiftKey) { selected = removeRange(index, selected, records); } else { @@ -116,6 +140,6 @@ export default () => { set(selectedSampleObjects, selectedObjects); setSelected(new Set(selected)); }, - [setSelected] + [records, setSelected] ); }; diff --git a/app/packages/core/src/components/Grid/useSpotlightPager.ts b/app/packages/core/src/components/Grid/useSpotlightPager.ts index 7b67fc0c457..a752139438a 100644 --- a/app/packages/core/src/components/Grid/useSpotlightPager.ts +++ b/app/packages/core/src/components/Grid/useSpotlightPager.ts @@ -14,6 +14,7 @@ import { import type { RecoilValueReadOnly } from "recoil"; import { useRecoilCallback, useRecoilValue } from "recoil"; import type { Subscription } from "relay-runtime"; +import type { Records } from "./useRecords"; export const PAGE_SIZE = 20; @@ -45,15 +46,18 @@ const processSamplePageData = ( }); }; -const useSpotlightPager = ({ - pageSelector, - zoomSelector, -}: { - pageSelector: RecoilValueReadOnly< - (page: number, pageSize: number) => VariablesOf - >; - zoomSelector: RecoilValueReadOnly; -}) => { +const useSpotlightPager = ( + { + pageSelector, + zoomSelector, + }: { + pageSelector: RecoilValueReadOnly< + (page: number, pageSize: number) => VariablesOf + >; + zoomSelector: RecoilValueReadOnly; + }, + records: Records +) => { const environment = useRelayEnvironment(); const pager = useRecoilValue(pageSelector); const zoom = useRecoilValue(zoomSelector); @@ -62,7 +66,8 @@ const useSpotlightPager = ({ () => new WeakMap(), [] ); - const records = useRef(new Map()); + + const keys = useRef(new Set()); const page = useRecoilCallback( ({ snapshot }) => { @@ -89,8 +94,9 @@ const useSpotlightPager = ({ data, schema, zoom, - records.current + records ); + for (const item of items) keys.current.add(item.id.description); resolve({ items, @@ -110,16 +116,16 @@ const useSpotlightPager = ({ ); const refresher = useRecoilValue(fos.refresher); + useEffect(() => { refresher; - const current = records.current; const clear = () => { commitLocalUpdate(fos.getCurrentEnvironment(), (store) => { - for (const id of Array.from(current.keys())) { + for (const id of keys.current) { store.get(id)?.invalidateRecord(); } - current.clear(); }); + keys.current.clear(); }; const unsubscribe = foq.subscribe( From dcb5570118231dcbe19563a0d1671460cda810a8 Mon Sep 17 00:00:00 2001 From: brimoor Date: Wed, 4 Sep 2024 00:51:40 -0400 Subject: [PATCH 2/6] adding 0.25.1 release notes --- docs/source/release-notes.rst | 63 +++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/docs/source/release-notes.rst b/docs/source/release-notes.rst index d175da62915..1978b44da66 100644 --- a/docs/source/release-notes.rst +++ b/docs/source/release-notes.rst @@ -3,6 +3,69 @@ FiftyOne Release Notes .. default-role:: code +FiftyOne Teams 2.0.1 +-------------------- +*Released September 6, 2024* + +Includes all updates from :ref:`FiftyOne 0.25.1 `, plus: + +- Optimized the `Manage > Access` page for datasets +- Added support for configuring a deployment to allow Guests to run custom + plugins +- Fixed a bug where dataset permissions assigned to + :ref:`groups ` were not correctly applied to users that do not + otherwise have access to the dataset +- Fixed a bug where a deployment's default user role as configured on the + `Security > Config` page would not be respected +- Fixed a bug that could cause 3D scenes stored in Azure to fail to load +- Fixed a bug that erroneously caused the currently selected samples to be + cleared when navigating between samples or closing the sample modal + +.. _release-notes-v0.25.1: + +FiftyOne 0.25.1 +--------------- +*Released September 6, 2024* + +App + +- Fixed an issue with sidebar state persistence when opening and closing the + sample modal + `#4745 `_ +- Fixed a bug with sample selection in the :ref:`Map panel ` + when the grid is reset + `#4739 `_ +- Fixed a bug when filtering |Keypoint| fields using the App sidebar + `#4735 `_ +- Fixed a bug when tagging in the sample modal with active sidebar filters + `#4723 `_ +- Disabled ``fiftyone-desktop`` builds until package size can be optimized + `#4746 `_ + +SDK + +- Added support for loading lists of TXT files in + :ref:`YOLOv5 format ` + `#4742 `_ +- Fixed a bug with the ``match_expr`` argument of + :meth:`group_by() ` + `#4754 `_ +- Fixed a regression when running inference with + :ref:`Ultralytics models ` that don't support track + IDs + `#4720 `_ + +Plugins + +- Fixed a bug that caused :class:`TabsView ` + components to erroneously reset to their default state + `#4732 `_ +- Fixed a bug where calling + :meth:`set_state() ` and + :meth:`set_data() ` to patch + state/data would inadvertently clobber other existing values + `#4753 `_ + FiftyOne Teams 2.0.0 -------------------- *Released August 20, 2024* From 52cf69ae746d9c4092d205b3ebaa99e217a527b3 Mon Sep 17 00:00:00 2001 From: brimoor Date: Thu, 22 Aug 2024 14:31:18 -0400 Subject: [PATCH 3/6] outputs may be None --- fiftyone/operators/delegated.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fiftyone/operators/delegated.py b/fiftyone/operators/delegated.py index 8e33a53af41..ff43dd597af 100644 --- a/fiftyone/operators/delegated.py +++ b/fiftyone/operators/delegated.py @@ -405,7 +405,8 @@ async def _execute_operator(self, doc): outputs = await resolve_type_with_context( request_params, "outputs" ) - outputs_schema = outputs.to_json() + if outputs is not None: + outputs_schema = outputs.to_json() except (AttributeError, Exception): logger.warning( "Failed to resolve output schema for the operation." From 9a4b0193306e1dfb58105174a5163ba1cc9c4d15 Mon Sep 17 00:00:00 2001 From: brimoor Date: Wed, 4 Sep 2024 10:08:24 -0400 Subject: [PATCH 4/6] adding release note --- docs/source/release-notes.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/source/release-notes.rst b/docs/source/release-notes.rst index 1978b44da66..14ffa9a0be9 100644 --- a/docs/source/release-notes.rst +++ b/docs/source/release-notes.rst @@ -65,6 +65,9 @@ Plugins :meth:`set_data() ` to patch state/data would inadvertently clobber other existing values `#4753 `_ +- Fixed a spurious warning that would appear for delegated operations that + don't return outputs + `#4715 `_ FiftyOne Teams 2.0.0 -------------------- From 3153c2f965f41be7a746b3f9c850b62858001b53 Mon Sep 17 00:00:00 2001 From: topher Date: Fri, 6 Sep 2024 18:07:33 +0000 Subject: [PATCH 5/6] bump version for next `develop` release --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 9a476e2eaff..23056b3886e 100644 --- a/setup.py +++ b/setup.py @@ -13,7 +13,7 @@ from setuptools import setup, find_packages -VERSION = "0.25.1" +VERSION = "1.0.0" def get_version(): From 06f617d96841929cd82d25843f00195a2122ec49 Mon Sep 17 00:00:00 2001 From: Sashank Aryal Date: Mon, 9 Sep 2024 15:14:19 -0500 Subject: [PATCH 6/6] hover mouse over tagger container to hide popout text that's intercepting click events --- e2e-pw/src/oss/poms/action-row/tagger/modal-tagger.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/e2e-pw/src/oss/poms/action-row/tagger/modal-tagger.ts b/e2e-pw/src/oss/poms/action-row/tagger/modal-tagger.ts index e999cebcb94..67cd571d752 100644 --- a/e2e-pw/src/oss/poms/action-row/tagger/modal-tagger.ts +++ b/e2e-pw/src/oss/poms/action-row/tagger/modal-tagger.ts @@ -9,6 +9,7 @@ export class ModalTaggerPom { } async toggleOpen() { + await this.locator.getByTestId("tagger-container").hover(); await this.modal.locator.getByTestId("action-tag-sample-labels").click(); }