Addressing multiple PCD overlay feedback (#2964)

* edit * copy * copy * rm log * wrong place * edit * copy * copy * rm log * wrong place * point clouds instead of pcds * edits * fixing make_optimized_view() * tweaking copy * fixing regression * removing unused parameter --------- Co-authored-by: brimoor <[email protected]> Co-authored-by: Sashank Aryal <[email protected]>
voxel51 · May 9, 2023 · 327789b · 327789b
1 parent 163cf4e
commit 327789b
Show file tree

Hide file tree

Showing 7 changed files with 34 additions and 79 deletions.
diff --git a/app/packages/core/src/components/Actions/GroupMediaVisibilityContainer.tsx b/app/packages/core/src/components/Actions/GroupMediaVisibilityContainer.tsx
@@ -1,7 +1,7 @@
 import { PillButton, PopoutSectionTitle } from "@fiftyone/components";
 import * as fos from "@fiftyone/state";
 import ViewComfyIcon from "@mui/icons-material/ViewComfy";
-import React, { useRef, useState } from "react";
+import { useRef, useState } from "react";
 import useMeasure from "react-use-measure";
 
 import {
@@ -20,7 +20,7 @@ interface GroupMediaVisibilityProps {
   modal: boolean;
 }
 
-const TITLE = "Toggle Media";
+const TITLE = "Toggle media";
 
 const Container = styled.div`
   position: relative;

diff --git a/app/packages/core/src/components/Actions/utils.tsx b/app/packages/core/src/components/Actions/utils.tsx
@@ -192,7 +192,7 @@ export const tagParameters = ({
     ...params,
     label_fields: activeFields,
     target_labels: targetLabels,
-    slices: !groups ? [groupData?.slices] : null,
+    slices: !groups ? groupData?.slices : null,
     group_id: params.modal ? groupData?.id : null,
     sample_ids: getSampleIds(),
     labels:

diff --git a/app/packages/core/src/components/Modal/Modal.tsx b/app/packages/core/src/components/Modal/Modal.tsx
@@ -133,7 +133,7 @@ const SampleModal = () => {
                     onBlur={() => {
                       controller.set({ zIndex: "0" });
                     }}
-                    disabled={isOther || isLabelTag}
+                    disabled={isOther || isLabelTag || isTag}
                     key={key}
                     trigger={trigger}
                   />

diff --git a/app/packages/core/src/components/Sidebar/Entries/EntryCounts.tsx b/app/packages/core/src/components/Sidebar/Entries/EntryCounts.tsx
@@ -57,7 +57,7 @@ export const PathEntryCounts = ({
   return shown.state === "loading" ? (
     <LoadingDots text="" />
   ) : (
-    typeof shown.contents === "number" && (
+    shown.contents && (
       <SuspenseEntryCounts
         countAtom={getAtom(false)}
         subcountAtom={getAtom(true)}

diff --git a/app/packages/looker-3d/src/action-bar/SliceSelector.tsx b/app/packages/looker-3d/src/action-bar/SliceSelector.tsx
@@ -18,15 +18,15 @@ export const SliceSelector = () => {
     }
 
     if (activePcdSlices.length === 1) {
-      return `${activePcdSlices[0]} selected`;
+      return `Showing ${activePcdSlices[0]}`;
     }
     if (activePcdSlices.length === 2) {
-      return activePcdSlices.join(" and ");
+      return `Showing ${activePcdSlices.join(" and ")}`;
     }
     if (activePcdSlices.length === allPcdSlices.length) {
-      return "All pcds selected";
+      return "Showing all point clouds";
     }
-    return `${activePcdSlices.length} point-clouds selected`;
+    return `Showing ${activePcdSlices.length} point clouds`;
   }, [activePcdSlices, allPcdSlices]);
 
   const handleActionClick = useCallback(() => {
@@ -43,7 +43,7 @@ export const SliceSelector = () => {
 
   return (
     <>
-      <ActionItem title="Select pcds">
+      <ActionItem title="Select point clouds">
         <div onClick={handleActionClick}>{activeSlicesLabel}</div>
       </ActionItem>
 
@@ -69,7 +69,7 @@ const PcdsSelector = () => {
 
   return (
     <ActionPopOver>
-      <PopoutSectionTitle>Select pcds</PopoutSectionTitle>
+      <PopoutSectionTitle>Select point clouds</PopoutSectionTitle>
       <div>
         {allPcdSlices.map((slice) => {
           return (

diff --git a/fiftyone/core/view.py b/fiftyone/core/view.py
@@ -1724,7 +1724,6 @@ def make_optimized_select_view(
     sample_ids,
     ordered=False,
     groups=False,
-    select_groups=False,
 ):
     """Returns a view that selects the provided sample IDs that is optimized
     to reduce the document list as early as possible in the pipeline.
@@ -1742,31 +1741,28 @@ def make_optimized_select_view(
         ordered (False): whether to sort the samples in the returned view to
             match the order of the provided IDs
         groups (False): whether the IDs are group IDs, not sample IDs
-        select_groups (False): whether to select sample groups via sample ids
 
     Returns:
         a :class:`DatasetView`
     """
-    view = sample_collection.view()
+    in_view = sample_collection.view()
+    media_type = sample_collection.media_type
+    stages = in_view._stages
 
-    if any(isinstance(stage, fost.Mongo) for stage in view._stages):
-        #
+    if any(isinstance(stage, fost.Mongo) for stage in stages):
         # We have no way of knowing what a `Mongo()` stage might do, so we must
         # run the entire view's aggregation first and then select the samples
         # of interest at the end
-        #
-        if groups:
-            return view.select_groups(sample_ids, ordered=ordered)
-        elif view.media_type == fom.GROUP and not select_groups:
-            return view.select_group_slices(_allow_mixed=True)
+        view = in_view
+        stages = []
+    else:
+        view = in_view._base_view
 
+    if groups:
+        view = view.select_groups(sample_ids, ordered=ordered)
+    else:
         view = view.select(sample_ids, ordered=ordered)
 
-        if view.media_type == fom.GROUP and select_groups:
-            view = view.select_group_slices(_allow_mixed=True)
-
-        return view
-
     #
     # Selecting the samples of interest first can be significantly faster than
     # running the entire aggregation and then selecting them.
@@ -1782,24 +1778,11 @@ def make_optimized_select_view(
     # we'll need to account for that here...
     #
 
-    optimized_view = view._base_view
-
-    if groups:
-        optimized_view = optimized_view.select_groups(
-            sample_ids, ordered=ordered
-        )
-    else:
-        optimized_view = optimized_view.select(sample_ids, ordered=ordered)
-        if view.media_type == fom.GROUP and select_groups:
-            optimized_view = optimized_view.select_group_slices(
-                _allow_mixed=True
-            )
-
-    for stage in view._stages:
+    for stage in stages:
         if type(stage) not in fost._STAGES_THAT_SELECT_OR_REORDER:
-            optimized_view._stages.append(stage)
+            view = view._add_view_stage(stage, validate=False)
 
-    return optimized_view
+    return view
 
 
 def _filter_schema(schema, selected_fields, excluded_fields):

diff --git a/tests/unittests/view_tests.py b/tests/unittests/view_tests.py
@@ -3733,41 +3733,10 @@ def test_view_field_copy(self):
         field = F("$ground_truth")
         self.assertEqual(str(field), str(deepcopy(field)))
 
-    def test_make_optimized_select_view_group_media_type_select_samples(self):
-        samples = self._create_group_samples()
-        dataset = self._create_group_dataset()
-        sample_ids = dataset.add_samples(samples)
-        self.assertEqual(len(sample_ids), len(samples))
-
-        # Default call should have select_groups = False
-        optimized_view = fov.make_optimized_select_view(dataset, sample_ids[0])
-
-        expected_stages = [fosg.Select(sample_ids[0])]
-        self.assertEqual(optimized_view._all_stages, expected_stages)
-
-    def test_make_optimized_select_view_group_media_type_select_groups(self):
-        samples = self._create_group_samples()
-        dataset = self._create_group_dataset()
-        sample_ids = dataset.add_samples(samples)
-        self.assertEqual(len(sample_ids), len(samples))
-
-        optimized_view = fov.make_optimized_select_view(
-            dataset, sample_ids[0], select_groups=True
-        )
-        expected_stages = [
-            fosg.Select(sample_ids[0]),
-            fosg.SelectGroupSlices(),
-        ]
-        self.assertEqual(optimized_view._all_stages, expected_stages)
-
-    def _create_group_dataset(self):
+    def test_make_optimized_select_view_group_dataset(self):
         dataset = fo.Dataset()
         dataset.add_group_field("group", default="center")
-        self.assertEqual(dataset.media_type, fom.GROUP)
-        self.assertEqual(dataset.default_group_slice, "center")
-        return dataset
 
-    def _create_group_samples(self):
         groups = ["left", "center", "right"]
         filepaths = [
             [str(i) + str(j) + ".jpg" for i in groups] for j in range(3)
@@ -3777,12 +3746,15 @@ def _create_group_samples(self):
         samples = []
         for fps in filepaths:
             for name, filepath in fps.items():
-                sample = fo.Sample(
-                    filepath=filepath, group=group.element(name)
+                samples.append(
+                    fo.Sample(filepath=filepath, group=group.element(name))
                 )
-                samples.append(sample)
-        assert all([s.group is not None for s in samples])
-        return samples
+
+        sample_ids = dataset.add_samples(samples)
+
+        optimized_view = fov.make_optimized_select_view(dataset, sample_ids[0])
+        expected_stages = [fosg.Select(sample_ids[0])]
+        self.assertEqual(optimized_view._all_stages, expected_stages)
 
 
 if __name__ == "__main__":