From 5ec3bd57448affe432499311a1267656ba887bca Mon Sep 17 00:00:00 2001 From: Kemal Eren Date: Thu, 29 Aug 2024 13:35:37 -0400 Subject: [PATCH 01/69] speed up evaluation with r-trees to find overlapping detections --- fiftyone/utils/eval/coco.py | 7 +- fiftyone/utils/eval/openimages.py | 5 +- fiftyone/utils/iou.py | 177 ++++++++++++++++++---------- fiftyone/utils/utils3d.py | 2 +- setup.py | 1 + tests/unittests/evaluation_tests.py | 10 +- tests/unittests/utils3d_tests.py | 11 ++ 7 files changed, 142 insertions(+), 71 deletions(-) diff --git a/fiftyone/utils/eval/coco.py b/fiftyone/utils/eval/coco.py index 8d495750ca..b9a4716c9b 100644 --- a/fiftyone/utils/eval/coco.py +++ b/fiftyone/utils/eval/coco.py @@ -527,12 +527,9 @@ def _coco_evaluation_setup( # Sort ground truth so crowds are last gts = sorted(gts, key=iscrowd) - # Compute ``num_preds x num_gts`` IoUs + # Compute IoUs for overlapping detections ious = foui.compute_ious(preds, gts, **iou_kwargs) - - gt_ids = [g.id for g in gts] - for pred, gt_ious in zip(preds, ious): - pred_ious[pred.id] = list(zip(gt_ids, gt_ious)) + pred_ious.update(ious) return cats, pred_ious, iscrowd diff --git a/fiftyone/utils/eval/openimages.py b/fiftyone/utils/eval/openimages.py index 70ef9db429..b7efff3894 100644 --- a/fiftyone/utils/eval/openimages.py +++ b/fiftyone/utils/eval/openimages.py @@ -564,10 +564,7 @@ def _open_images_evaluation_setup( # Compute ``num_preds x num_gts`` IoUs ious = foui.compute_ious(preds, gts, **iou_kwargs) - - gt_ids = [g.id for g in gts] - for pred, gt_ious in zip(preds, ious): - pred_ious[pred.id] = list(zip(gt_ids, gt_ious)) + pred_ious.update(ious) return cats, pred_ious, iscrowd diff --git a/fiftyone/utils/iou.py b/fiftyone/utils/iou.py index 698b41b210..6d3e9c9e3f 100644 --- a/fiftyone/utils/iou.py +++ b/fiftyone/utils/iou.py @@ -11,6 +11,7 @@ import numpy as np import scipy.spatial as sp +import rtree.index as rti import eta.core.numutils as etan import eta.core.utils as etau @@ -19,7 +20,7 @@ import fiftyone.core.utils as fou import fiftyone.core.validation as fov -from .utils3d import compute_cuboid_iou +from .utils3d import compute_cuboid_iou, _Box sg = fou.lazy_import("shapely.geometry") so = fou.lazy_import("shapely.ops") @@ -84,8 +85,11 @@ def compute_ious( Returns: a ``num_preds x num_gts`` array of IoUs """ - if not preds or not gts: - return np.zeros((len(preds), len(gts))) + if not preds: + return {} + + if not gts: + return dict((p.id, []) for p in preds) if etau.is_str(iscrowd): iscrowd = lambda l: bool(l.get_attribute_value(iscrowd, False)) @@ -485,6 +489,37 @@ def compute_bbox_iou(gt, pred, gt_crowd=False): return min(etan.safe_divide(inter, union), 1) +def _get_detection_box(det, dimension=None): + if dimension is None: + dimension = _get_bbox_dim(det) + + if dimension == 2: + px, py, pw, ph = det.bounding_box + xmin = px + xmax = px + pw + ymin = py + ymax = py + ph + result = (xmin, xmax, ymin, ymax) + elif dimension == 3: + box = _Box(det.rotation, det.location, det.dimensions) + xmin = np.min(box.vertices[:, 0]) + xmax = np.max(box.vertices[:, 0]) + ymin = np.min(box.vertices[:, 1]) + ymax = np.max(box.vertices[:, 1]) + zmin = np.min(box.vertices[:, 2]) + zmax = np.max(box.vertices[:, 2]) + result = (xmin, xmax, ymin, ymax, zmin, zmax) + else: + raise Exception(f"dimension should be 2 or 3, but got {dimension}") + + return result + + +def _get_poly_box(x): + detection = x.to_detection() + return _get_detection_box(detection) + + def _compute_bbox_ious(preds, gts, iscrowd=None, classwise=False): is_symmetric = preds is gts @@ -501,28 +536,36 @@ def _compute_bbox_ious(preds, gts, iscrowd=None, classwise=False): else: gts = _polylines_to_detections(gts) + index_property = rti.Property() if _get_bbox_dim(gts[0]) == 3: bbox_iou_fcn = compute_cuboid_iou + index_property.dimension = 3 else: bbox_iou_fcn = compute_bbox_iou - - ious = np.zeros((len(preds), len(gts))) - - for j, (gt, gt_crowd) in enumerate(zip(gts, gt_crowds)): - - for i, pred in enumerate(preds): - if is_symmetric and i < j: - iou = ious[j, i] - elif is_symmetric and i == j: - iou = 1 - elif classwise and pred.label != gt.label: + index_property.dimension = 2 + + rtree_index = rti.Index(properties=index_property, interleaved=False) + for i, gt in enumerate(gts): + box = _get_detection_box(gt, dimension=index_property.dimension) + rtree_index.insert(i, box) + + pred_ious = {} + for i, pred in enumerate(preds): + box = _get_detection_box(pred, dimension=index_property.dimension) + indices = rtree_index.intersection(box) + pred_ious[pred.id] = [] + for j in indices: # pylint: disable=not-an-iterable + gt = gts[j] + gt_crowd = gt_crowds[j] + if classwise and pred.label != gt.label: continue - else: - iou = bbox_iou_fcn(gt, pred, gt_crowd=gt_crowd) - ious[i, j] = iou + iou = bbox_iou_fcn(gt, pred, gt_crowd=gt_crowd) + pred_ious[pred.id].append((gt.id, iou)) + if is_symmetric: + pred_ious[gt.id].append((pred.id, iou)) - return ious + return pred_ious def _compute_polygon_ious( @@ -564,57 +607,71 @@ def _compute_polygon_ious( elif gt_crowds is None: gt_crowds = [False] * num_gt - ious = np.zeros((num_pred, num_gt)) + rtree_index = rti.Index(interleaved=False) + for i, gt in enumerate(gts): + box = _get_poly_box(gt) + rtree_index.insert(i, box) - for j, (gt_poly, gt_label, gt_area, gt_crowd) in enumerate( - zip(gt_polys, gt_labels, gt_areas, gt_crowds) + pred_ious = {} + for i, (pred, pred_poly, pred_label, pred_area) in enumerate( + zip(preds, pred_polys, pred_labels, pred_areas) ): - for i, (pred_poly, pred_label, pred_area) in enumerate( - zip(pred_polys, pred_labels, pred_areas) - ): - if is_symmetric and i < j: - iou = ious[j, i] - elif is_symmetric and i == j: - iou = 1 - elif classwise and pred_label != gt_label: + pred_ious[pred.id] = [] + + if is_symmetric: + pred_ious[pred.id].append((pred.id, 1)) + + box = _get_poly_box(pred) + indices = rtree_index.intersection(box) + for j in indices: # pylint: disable=not-an-iterable + gt = gts[j] + gt_poly = gt_polys[j] + gt_label = gt_labels[j] + gt_area = gt_areas[j] + gt_crowd = gt_crowds[j] + + if classwise and pred_label != gt_label: continue - else: - try: - inter = gt_poly.intersection(pred_poly).area - except Exception as e: - inter = 0.0 - fou.handle_error( - ValueError( - "Failed to compute intersection of predicted " - "object '%s' and ground truth object '%s'" - % (preds[i].id, gts[j].id) - ), - error_level, - base_error=e, - ) - - if gt_crowd: - union = pred_area - else: - union = pred_area + gt_area - inter - iou = min(etan.safe_divide(inter, union), 1) + try: + inter = gt_poly.intersection(pred_poly).area + except Exception as e: + inter = 0.0 + fou.handle_error( + ValueError( + "Failed to compute intersection of predicted " + "object '%s' and ground truth object '%s'" + % (preds[i].id, gts[j].id) + ), + error_level, + base_error=e, + ) + + if gt_crowd: + union = pred_area + else: + union = pred_area + gt_area - inter - ious[i, j] = iou + iou = min(etan.safe_divide(inter, union), 1) + pred_ious[pred.id].append((gt.id, iou)) + if is_symmetric: + pred_ious[gt.id].append((pred.id, iou)) - return ious + return pred_ious def _compute_polyline_similarities(preds, gts, classwise=False): - sims = np.zeros((len(preds), len(gts))) - for j, gt in enumerate(gts): - for i, pred in enumerate(preds): + sims = {} + for pred in preds: + sims[pred.id] == [] + for gt in gts: if classwise and pred.label != gt.label: continue gtp = list(itertools.chain.from_iterable(gt.points)) predp = list(itertools.chain.from_iterable(pred.points)) - sims[i, j] = _compute_object_keypoint_similarity(gtp, predp) + sim = _compute_object_keypoint_similarity(gtp, predp) + sims[pred.id].append((gt.id, sim)) return sims @@ -687,15 +744,17 @@ def _compute_segment_ious(preds, gts): def _compute_keypoint_similarities(preds, gts, classwise=False): - sims = np.zeros((len(preds), len(gts))) - for j, gt in enumerate(gts): - for i, pred in enumerate(preds): + sims = {} + for pred in preds: + sims[pred.id] = [] + for gt in gts: if classwise and pred.label != gt.label: continue gtp = gt.points predp = pred.points - sims[i, j] = _compute_object_keypoint_similarity(gtp, predp) + sim = _compute_object_keypoint_similarity(gtp, predp) + sims[pred.id].append((gt.id, sim)) return sims diff --git a/fiftyone/utils/utils3d.py b/fiftyone/utils/utils3d.py index 1aebb13bef..c48edcfd37 100644 --- a/fiftyone/utils/utils3d.py +++ b/fiftyone/utils/utils3d.py @@ -167,7 +167,7 @@ def __init__(self, rotation, location, scale): vertices = np.zeros((_NUM_KEYPOINTS, 3)) for i in range(_NUM_KEYPOINTS): vertices[i, :] = ( - np.matmul(rotation, scaled_identity_box[i, :]) + np.matmul(self.rotation, scaled_identity_box[i, :]) + location.flatten() ) diff --git a/setup.py b/setup.py index b2bf8f25dd..b701073e54 100644 --- a/setup.py +++ b/setup.py @@ -60,6 +60,7 @@ def get_version(): "PyYAML", "regex", "retrying", + "rtree", "scikit-learn", "scikit-image", "scipy", diff --git a/tests/unittests/evaluation_tests.py b/tests/unittests/evaluation_tests.py index 130c64c32c..d7dc34de7a 100644 --- a/tests/unittests/evaluation_tests.py +++ b/tests/unittests/evaluation_tests.py @@ -1936,9 +1936,15 @@ def _make_box(self, dimensions, location, rotation): def _check_iou(self, dataset, field1, field2, expected_iou): dets1 = dataset.first()[field1].detections dets2 = dataset.first()[field2].detections - actual_iou = foui.compute_ious(dets1, dets2)[0][0] + ious = foui.compute_ious(dets1, dets2) + result = list(ious.values())[0] - self.assertTrue(np.isclose(actual_iou, expected_iou)) + if expected_iou == 0: + self.assertTrue(len(result) == 0) + + else: + _, actual_iou = result[0] + self.assertTrue(np.isclose(actual_iou, expected_iou)) @drop_datasets def test_non_overlapping_boxes(self): diff --git a/tests/unittests/utils3d_tests.py b/tests/unittests/utils3d_tests.py index a949ade397..964f09978b 100644 --- a/tests/unittests/utils3d_tests.py +++ b/tests/unittests/utils3d_tests.py @@ -10,6 +10,7 @@ import unittest import numpy as np +import numpy.testing as nptest import open3d as o3d from PIL import Image @@ -389,6 +390,16 @@ def test_get_scene_asset_paths(self): ) +class BoxTests(unittest.TestCase): + def test_box_vertices(self): + rotation = [0, 0, 0] + location = [0, 0, 0] + scale = [1, 1, 1] + box = fou3d._Box(rotation, location, scale) + expected = fou3d._UNIT_BOX + nptest.assert_equal(expected, box.vertices) + + if __name__ == "__main__": fo.config.show_progress_bars = False unittest.main(verbosity=2) From 02d7fdea721f5bef4211fa51f7d50fa6bfbf2683 Mon Sep 17 00:00:00 2001 From: Kemal Eren Date: Fri, 30 Aug 2024 12:52:03 -0400 Subject: [PATCH 02/69] fix assignment to use '=' instead of '==' --- fiftyone/utils/iou.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fiftyone/utils/iou.py b/fiftyone/utils/iou.py index 6d3e9c9e3f..994c918a23 100644 --- a/fiftyone/utils/iou.py +++ b/fiftyone/utils/iou.py @@ -663,7 +663,7 @@ def _compute_polygon_ious( def _compute_polyline_similarities(preds, gts, classwise=False): sims = {} for pred in preds: - sims[pred.id] == [] + sims[pred.id] = [] for gt in gts: if classwise and pred.label != gt.label: continue From e7223af739fc199cf94f223e571254dc424159fe Mon Sep 17 00:00:00 2001 From: brimoor Date: Fri, 4 Oct 2024 23:53:44 +0200 Subject: [PATCH 03/69] backwards compatibility --- fiftyone/utils/eval/coco.py | 2 +- fiftyone/utils/eval/openimages.py | 2 +- fiftyone/utils/iou.py | 264 ++++++++++++++++++++-------- tests/unittests/evaluation_tests.py | 92 +++++++++- 4 files changed, 280 insertions(+), 80 deletions(-) diff --git a/fiftyone/utils/eval/coco.py b/fiftyone/utils/eval/coco.py index b9a4716c9b..1efa89331b 100644 --- a/fiftyone/utils/eval/coco.py +++ b/fiftyone/utils/eval/coco.py @@ -528,7 +528,7 @@ def _coco_evaluation_setup( gts = sorted(gts, key=iscrowd) # Compute IoUs for overlapping detections - ious = foui.compute_ious(preds, gts, **iou_kwargs) + ious = foui.compute_ious(preds, gts, sparse=True, **iou_kwargs) pred_ious.update(ious) return cats, pred_ious, iscrowd diff --git a/fiftyone/utils/eval/openimages.py b/fiftyone/utils/eval/openimages.py index b7efff3894..ffd4d65e20 100644 --- a/fiftyone/utils/eval/openimages.py +++ b/fiftyone/utils/eval/openimages.py @@ -563,7 +563,7 @@ def _open_images_evaluation_setup( gts = sorted(gts, key=iscrowd) # Compute ``num_preds x num_gts`` IoUs - ious = foui.compute_ious(preds, gts, **iou_kwargs) + ious = foui.compute_ious(preds, gts, sparse=True, **iou_kwargs) pred_ious.update(ious) return cats, pred_ious, iscrowd diff --git a/fiftyone/utils/iou.py b/fiftyone/utils/iou.py index 994c918a23..53488eebc4 100644 --- a/fiftyone/utils/iou.py +++ b/fiftyone/utils/iou.py @@ -5,6 +5,7 @@ | `voxel51.com `_ | """ +from collections import defaultdict import contextlib import itertools import logging @@ -34,6 +35,7 @@ def compute_ious( use_masks=False, use_boxes=False, tolerance=None, + sparse=False, error_level=1, ): """Computes the pairwise IoUs between the predicted and ground truth @@ -72,6 +74,8 @@ def compute_ious( rather than using their actual geometries tolerance (None): a tolerance, in pixels, when generating approximate polylines for instance masks. Typical values are 1-3 pixels + sparse (False): whether to return a sparse dict of non-zero IoUs rather + than a full matrix error_level (1): the error level to use when manipulating instance masks or polylines. Valid values are: @@ -83,13 +87,14 @@ def compute_ious( error will default to an IoU of 0 Returns: - a ``num_preds x num_gts`` array of IoUs + a ``num_preds x num_gts`` array of IoUs when ``sparse=False`, or a dict + of the form ``d[pred.id] = [(gt.id, iou), ...]`` when ``sparse=True`` """ - if not preds: - return {} - - if not gts: - return dict((p.id, []) for p in preds) + if not preds or not gts: + if sparse: + return {p.id: [] for p in preds} if preds else {} + else: + return np.zeros((len(preds or []), len(gts or []))) if etau.is_str(iscrowd): iscrowd = lambda l: bool(l.get_attribute_value(iscrowd, False)) @@ -97,18 +102,37 @@ def compute_ious( if isinstance(gts[0], fol.Polyline): if use_boxes: return _compute_bbox_ious( - preds, gts, iscrowd=iscrowd, classwise=classwise + preds, + gts, + iscrowd=iscrowd, + classwise=classwise, + sparse=sparse, ) if any(gt.filled for gt in gts): return _compute_polygon_ious( - preds, gts, error_level, iscrowd=iscrowd, classwise=classwise + preds, + gts, + error_level, + iscrowd=iscrowd, + classwise=classwise, + sparse=sparse, ) - return _compute_polyline_similarities(preds, gts, classwise=classwise) + return _compute_polyline_similarities( + preds, + gts, + classwise=classwise, + sparse=sparse, + ) if isinstance(gts[0], fol.Keypoint): - return _compute_keypoint_similarities(preds, gts, classwise=classwise) + return _compute_keypoint_similarities( + preds, + gts, + classwise=classwise, + sparse=sparse, + ) if use_masks: # @todo when tolerance is None, consider using dense masks rather than @@ -123,12 +147,19 @@ def compute_ious( error_level, iscrowd=iscrowd, classwise=classwise, + sparse=sparse, ) - return _compute_bbox_ious(preds, gts, iscrowd=iscrowd, classwise=classwise) + return _compute_bbox_ious( + preds, + gts, + iscrowd=iscrowd, + classwise=classwise, + sparse=sparse, + ) -def compute_segment_ious(preds, gts): +def compute_segment_ious(preds, gts, sparse=False): """Computes the pairwise IoUs between the predicted and ground truth temporal detections. @@ -137,14 +168,21 @@ def compute_segment_ious(preds, gts): :class:`fiftyone.core.labels.TemporalDetection` instances gts: a list of ground truth :class:`fiftyone.core.labels.TemporalDetection` instances + sparse (False): whether to return a sparse dict of non-zero IoUs rather + than a full matrix Returns: - a ``num_preds x num_gts`` array of segment IoUs + a ``num_preds x num_gts`` array of segment IoUs when ``sparse=False`, + or a dict of the form ``d[pred.id] = [(gt.id, iou), ...]`` when + ``sparse=True`` """ if not preds or not gts: - return np.zeros((len(preds), len(gts))) + if sparse: + return {p.id: [] for p in preds} + else: + return np.zeros((len(preds), len(gts))) - return _compute_segment_ious(preds, gts) + return _compute_segment_ious(preds, gts, sparse=sparse) def compute_max_ious( @@ -520,9 +558,20 @@ def _get_poly_box(x): return _get_detection_box(detection) -def _compute_bbox_ious(preds, gts, iscrowd=None, classwise=False): +def _compute_bbox_ious( + preds, + gts, + iscrowd=None, + classwise=False, + sparse=False, +): is_symmetric = preds is gts + if sparse: + ious = defaultdict(list) + else: + ious = np.zeros((len(preds), len(gts))) + if iscrowd is not None: gt_crowds = [iscrowd(gt) for gt in gts] else: @@ -530,7 +579,6 @@ def _compute_bbox_ious(preds, gts, iscrowd=None, classwise=False): if isinstance(preds[0], fol.Polyline): preds = _polylines_to_detections(preds) - if is_symmetric: gts = preds else: @@ -549,23 +597,30 @@ def _compute_bbox_ious(preds, gts, iscrowd=None, classwise=False): box = _get_detection_box(gt, dimension=index_property.dimension) rtree_index.insert(i, box) - pred_ious = {} for i, pred in enumerate(preds): box = _get_detection_box(pred, dimension=index_property.dimension) indices = rtree_index.intersection(box) - pred_ious[pred.id] = [] for j in indices: # pylint: disable=not-an-iterable gt = gts[j] gt_crowd = gt_crowds[j] if classwise and pred.label != gt.label: continue + if is_symmetric and j > i: + continue + iou = bbox_iou_fcn(gt, pred, gt_crowd=gt_crowd) - pred_ious[pred.id].append((gt.id, iou)) - if is_symmetric: - pred_ious[gt.id].append((pred.id, iou)) - return pred_ious + if sparse: + ious[pred.id].append((gt.id, iou)) + if is_symmetric: + ious[gt.id].append((pred.id, iou)) + else: + ious[i, j] = iou + if is_symmetric: + ious[j, i] = iou + + return ious def _compute_polygon_ious( @@ -575,9 +630,15 @@ def _compute_polygon_ious( iscrowd=None, classwise=False, gt_crowds=None, + sparse=False, ): is_symmetric = preds is gts + if sparse: + ious = defaultdict(list) + else: + ious = np.zeros((len(preds), len(gts))) + with contextlib.ExitStack() as context: # We're ignoring errors, so suppress shapely logging that occurs when # invalid geometries are encountered @@ -586,18 +647,15 @@ def _compute_polygon_ious( fou.LoggingLevel(logging.CRITICAL, logger="shapely") ) - num_pred = len(preds) pred_polys = _polylines_to_shapely(preds, error_level) pred_labels = [pred.label for pred in preds] pred_areas = [pred_poly.area for pred_poly in pred_polys] if is_symmetric: - num_gt = num_pred gt_polys = pred_polys gt_labels = pred_labels gt_areas = pred_areas else: - num_gt = len(gts) gt_polys = _polylines_to_shapely(gts, error_level) gt_labels = [gt.label for gt in gts] gt_areas = [gt_poly.area for gt_poly in gt_polys] @@ -605,22 +663,16 @@ def _compute_polygon_ious( if iscrowd is not None: gt_crowds = [iscrowd(gt) for gt in gts] elif gt_crowds is None: - gt_crowds = [False] * num_gt + gt_crowds = [False] * len(gts) rtree_index = rti.Index(interleaved=False) for i, gt in enumerate(gts): box = _get_poly_box(gt) rtree_index.insert(i, box) - pred_ious = {} for i, (pred, pred_poly, pred_label, pred_area) in enumerate( zip(preds, pred_polys, pred_labels, pred_areas) ): - pred_ious[pred.id] = [] - - if is_symmetric: - pred_ious[pred.id].append((pred.id, 1)) - box = _get_poly_box(pred) indices = rtree_index.intersection(box) for j in indices: # pylint: disable=not-an-iterable @@ -633,10 +685,12 @@ def _compute_polygon_ious( if classwise and pred_label != gt_label: continue + if is_symmetric and j > i: + continue + try: inter = gt_poly.intersection(pred_poly).area except Exception as e: - inter = 0.0 fou.handle_error( ValueError( "Failed to compute intersection of predicted " @@ -646,6 +700,7 @@ def _compute_polygon_ious( error_level, base_error=e, ) + continue if gt_crowd: union = pred_area @@ -653,31 +708,61 @@ def _compute_polygon_ious( union = pred_area + gt_area - inter iou = min(etan.safe_divide(inter, union), 1) - pred_ious[pred.id].append((gt.id, iou)) - if is_symmetric: - pred_ious[gt.id].append((pred.id, iou)) - return pred_ious + if sparse: + ious[pred.id].append((gt.id, iou)) + if is_symmetric: + ious[gt.id].append((pred.id, iou)) + else: + ious[i, j] = iou + if is_symmetric: + ious[j, i] = iou + + return ious -def _compute_polyline_similarities(preds, gts, classwise=False): - sims = {} - for pred in preds: - sims[pred.id] = [] - for gt in gts: +def _compute_polyline_similarities(preds, gts, classwise=False, sparse=False): + is_symmetric = preds is gts + + if sparse: + sims = defaultdict(list) + else: + sims = np.zeros((len(preds), len(gts))) + + for i, pred in enumerate(preds): + for j, gt in enumerate(gts): if classwise and pred.label != gt.label: continue + if is_symmetric and j > i: + continue + gtp = list(itertools.chain.from_iterable(gt.points)) predp = list(itertools.chain.from_iterable(pred.points)) sim = _compute_object_keypoint_similarity(gtp, predp) - sims[pred.id].append((gt.id, sim)) + if sim <= 0: + continue + + if sparse: + sims[pred.id].append((gt.id, sim)) + if is_symmetric: + sims[gt.id].append((pred.id, sim)) + else: + sims[i, j] = sim + if is_symmetric: + sims[j, i] = sim return sims def _compute_mask_ious( - preds, gts, tolerance, error_level, iscrowd=None, classwise=False + preds, + gts, + tolerance, + error_level, + iscrowd=None, + classwise=False, + sparse=False, ): is_symmetric = preds is gts @@ -707,54 +792,85 @@ def _compute_mask_ious( error_level, classwise=classwise, gt_crowds=gt_crowds, + sparse=sparse, ) -def _compute_segment_ious(preds, gts): +def _compute_segment_ious(preds, gts, sparse=False): is_symmetric = preds is gts - ious = np.zeros((len(preds), len(gts))) - for j, gt in enumerate(gts): - gst, get = gt.support - gt_len = get - gst + if sparse: + ious = defaultdict(list) + else: + ious = np.zeros((len(preds), len(gts))) + + for i, pred in enumerate(preds): + for j, gt in enumerate(gts): + if is_symmetric and j > i: + continue - for i, pred in enumerate(preds): - if is_symmetric and i < j: - iou = ious[j, i] - elif is_symmetric and i == j: - iou = 1 - else: - pst, pet = pred.support - pred_len = pet - pst + gst, get = gt.support + gt_len = get - gst - if pred_len == 0 and gt_len == 0: - iou = float(pet == get) - else: - # Length of temporal intersection - inter = min(get, pet) - max(gst, pst) - if inter <= 0: - continue + pst, pet = pred.support + pred_len = pet - pst - union = pred_len + gt_len - inter - iou = min(etan.safe_divide(inter, union), 1) + if pred_len == 0 and gt_len == 0: + if pet != get: + continue + + iou = 1.0 + else: + # Length of temporal intersection + inter = min(get, pet) - max(gst, pst) + if inter <= 0: + continue - ious[i, j] = iou + union = pred_len + gt_len - inter + iou = min(etan.safe_divide(inter, union), 1) + + if sparse: + ious[pred.id].append((gt.id, iou)) + if is_symmetric: + ious[gt.id].append((pred.id, iou)) + else: + ious[i, j] = iou + if is_symmetric: + ious[j, i] = iou return ious -def _compute_keypoint_similarities(preds, gts, classwise=False): - sims = {} - for pred in preds: - sims[pred.id] = [] - for gt in gts: +def _compute_keypoint_similarities(preds, gts, classwise=False, sparse=False): + is_symmetric = preds is gts + + if sparse: + sims = defaultdict(list) + else: + sims = np.zeros((len(preds), len(gts))) + + for i, pred in enumerate(preds): + for j, gt in enumerate(gts): if classwise and pred.label != gt.label: continue + if is_symmetric and j > i: + continue + gtp = gt.points predp = pred.points sim = _compute_object_keypoint_similarity(gtp, predp) - sims[pred.id].append((gt.id, sim)) + if sim <= 0: + continue + + if sparse: + sims[pred.id].append((gt.id, sim)) + if is_symmetric: + sims[gt.id].append((pred.id, sim)) + else: + sims[i, j] = sim + if is_symmetric: + sims[j, i] = sim return sims diff --git a/tests/unittests/evaluation_tests.py b/tests/unittests/evaluation_tests.py index d7dc34de7a..3ffcb9b977 100644 --- a/tests/unittests/evaluation_tests.py +++ b/tests/unittests/evaluation_tests.py @@ -1845,6 +1845,91 @@ def test_custom_detection_evaluation(self): self.assertEqual(type(results), foud.DetectionResults) +class BoxesTests(unittest.TestCase): + def _make_dataset(self): + dataset = fo.Dataset() + + sample1 = fo.Sample( + filepath="image2.jpg", + ground_truth=fo.Detections( + detections=[ + fo.Detection( + label="cat", + bounding_box=[0.1, 0.1, 0.4, 0.4], + ), + fo.Detection( + label="dog", + bounding_box=[0.11, 0.11, 0.39, 0.39], + ), + ] + ), + predictions=fo.Detections( + detections=[ + fo.Detection( + label="cat", + bounding_box=[0.1, 0.1, 0.4, 0.4], + confidence=0.9, + ), + fo.Detection( + label="dog", + bounding_box=[0.11, 0.11, 0.39, 0.39], + confidence=0.9, + ), + ] + ), + ) + sample2 = fo.Sample(filepath="image1.jpg") + + dataset.add_samples([sample1, sample2]) + + return dataset + + def test_compute_max_ious(self): + dataset = self._make_dataset() + + foui.compute_max_ious( + dataset, + "ground_truth", + iou_attr="max_iou", + ) + bounds1 = dataset.bounds("ground_truth.detections.max_iou") + + self.assertIsNotNone(bounds1[0]) + self.assertIsNotNone(bounds1[1]) + + foui.compute_max_ious( + dataset, + "predictions", + other_field="ground_truth", + iou_attr="max_iou", + ) + bounds2 = dataset.bounds("predictions.detections.max_iou") + + self.assertIsNotNone(bounds2[0]) + self.assertIsNotNone(bounds2[1]) + + def test_find_duplicates(self): + dataset = self._make_dataset() + + dup_ids1 = foui.find_duplicates( + dataset, + "ground_truth", + iou_thresh=0.9, + method="simple", + ) + + self.assertEqual(len(dup_ids1), 1) + + dup_ids2 = foui.find_duplicates( + dataset, + "ground_truth", + iou_thresh=0.9, + method="greedy", + ) + + self.assertEqual(len(dup_ids2), 1) + + class CuboidTests(unittest.TestCase): def _make_dataset(self): group = fo.Group() @@ -1936,12 +2021,11 @@ def _make_box(self, dimensions, location, rotation): def _check_iou(self, dataset, field1, field2, expected_iou): dets1 = dataset.first()[field1].detections dets2 = dataset.first()[field2].detections - ious = foui.compute_ious(dets1, dets2) - result = list(ious.values())[0] + ious = foui.compute_ious(dets1, dets2, sparse=True) + result = next(iter(ious.values()), []) if expected_iou == 0: self.assertTrue(len(result) == 0) - else: _, actual_iou = result[0] self.assertTrue(np.isclose(actual_iou, expected_iou)) @@ -1950,7 +2034,7 @@ def _check_iou(self, dataset, field1, field2, expected_iou): def test_non_overlapping_boxes(self): dataset = self._make_dataset() - expected_iou = 0.0 + expected_iou = 0 self._check_iou(dataset, "test1_box1", "test1_box2", expected_iou) @drop_datasets From b0b93e84067d58e043ac94e0ff2fcba3de9aa194 Mon Sep 17 00:00:00 2001 From: brimoor Date: Sat, 5 Oct 2024 00:00:27 +0200 Subject: [PATCH 04/69] linting --- fiftyone/utils/iou.py | 4 ++-- tests/unittests/evaluation_tests.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/fiftyone/utils/iou.py b/fiftyone/utils/iou.py index 53488eebc4..3d838c5f22 100644 --- a/fiftyone/utils/iou.py +++ b/fiftyone/utils/iou.py @@ -11,8 +11,8 @@ import logging import numpy as np -import scipy.spatial as sp import rtree.index as rti +import scipy.spatial as sp import eta.core.numutils as etan import eta.core.utils as etau @@ -180,7 +180,7 @@ def compute_segment_ious(preds, gts, sparse=False): if sparse: return {p.id: [] for p in preds} else: - return np.zeros((len(preds), len(gts))) + return np.zeros((len(preds or []), len(gts or []))) return _compute_segment_ious(preds, gts, sparse=sparse) diff --git a/tests/unittests/evaluation_tests.py b/tests/unittests/evaluation_tests.py index 3ffcb9b977..e9c56bd01f 100644 --- a/tests/unittests/evaluation_tests.py +++ b/tests/unittests/evaluation_tests.py @@ -1850,7 +1850,7 @@ def _make_dataset(self): dataset = fo.Dataset() sample1 = fo.Sample( - filepath="image2.jpg", + filepath="image1.jpg", ground_truth=fo.Detections( detections=[ fo.Detection( @@ -1878,7 +1878,7 @@ def _make_dataset(self): ] ), ) - sample2 = fo.Sample(filepath="image1.jpg") + sample2 = fo.Sample(filepath="image2.jpg") dataset.add_samples([sample1, sample2]) From 2c12573643fc1d5aec02c1f0b7944001edfebe44 Mon Sep 17 00:00:00 2001 From: Jacob Marks Date: Sat, 5 Oct 2024 11:50:09 -0400 Subject: [PATCH 05/69] rm unused import --- fiftyone/utils/ultralytics.py | 1 - 1 file changed, 1 deletion(-) diff --git a/fiftyone/utils/ultralytics.py b/fiftyone/utils/ultralytics.py index c5473a669d..6eafa7eab2 100644 --- a/fiftyone/utils/ultralytics.py +++ b/fiftyone/utils/ultralytics.py @@ -17,7 +17,6 @@ from fiftyone.core.models import Model import fiftyone.utils.torch as fout import fiftyone.core.utils as fou -import fiftyone.zoo as foz import fiftyone.zoo.models as fozm ultralytics = fou.lazy_import("ultralytics") From 77d8436f46a32c02b30e5cd04cd9207dca95a480 Mon Sep 17 00:00:00 2001 From: Jacob Marks Date: Sat, 5 Oct 2024 11:51:26 -0400 Subject: [PATCH 06/69] fix yolonas tag --- fiftyone/zoo/models/manifest-torch.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fiftyone/zoo/models/manifest-torch.json b/fiftyone/zoo/models/manifest-torch.json index 468d4d8004..c75c176f86 100644 --- a/fiftyone/zoo/models/manifest-torch.json +++ b/fiftyone/zoo/models/manifest-torch.json @@ -3796,7 +3796,7 @@ "support": true } }, - "tags": ["classification", "torch", "yolo"], + "tags": ["detection", "torch", "yolo"], "date_added": "2024-01-06 08:51:14" }, { From 9b845805c183979e8ca27419d396a7fd53e04399 Mon Sep 17 00:00:00 2001 From: Jacob Marks Date: Sat, 5 Oct 2024 12:22:51 -0400 Subject: [PATCH 07/69] add yolov11 det models to zoo --- fiftyone/zoo/models/manifest-torch.json | 160 ++++++++++++++++++++++++ 1 file changed, 160 insertions(+) diff --git a/fiftyone/zoo/models/manifest-torch.json b/fiftyone/zoo/models/manifest-torch.json index c75c176f86..1d748dcda8 100644 --- a/fiftyone/zoo/models/manifest-torch.json +++ b/fiftyone/zoo/models/manifest-torch.json @@ -3260,6 +3260,166 @@ "tags": ["detection", "coco", "torch", "yolo"], "date_added": "2024-07-01 19:22:51" }, + { + "base_name": "yolov11n-coco-torch", + "base_filename": "yolov11n-coco.pt", + "description": "YOLOv11-N model trained on COCO", + "source": "https://docs.ultralytics.com/models/yolov11/", + "size_bytes": 5613764, + "manager": { + "type": "fiftyone.core.models.ModelManager", + "config": { + "url": "https://github.com/ultralytics/assets/releases/download/v8.3.0/yolo11n.pt" + } + }, + "default_deployment_config_dict": { + "type": "fiftyone.utils.ultralytics.FiftyOneYOLODetectionModel", + "config": {} + }, + "requirements": { + "packages": [ + "torch>=1.7.0", + "torchvision>=0.8.1", + "ultralytics>=8.3.0" + ], + "cpu": { + "support": true + }, + "gpu": { + "support": true + } + }, + "tags": ["detection", "coco", "torch", "yolo"], + "date_added": "2024-10-05 19:22:51" + }, + { + "base_name": "yolov11s-coco-torch", + "base_filename": "yolov11s-coco.pt", + "description": "YOLOv11-S model trained on COCO", + "source": "https://docs.ultralytics.com/models/yolov11/", + "size_bytes": 19313732, + "manager": { + "type": "fiftyone.core.models.ModelManager", + "config": { + "url": "https://github.com/ultralytics/assets/releases/download/v8.3.0/yolo11s.pt" + } + }, + "default_deployment_config_dict": { + "type": "fiftyone.utils.ultralytics.FiftyOneYOLODetectionModel", + "config": {} + }, + "requirements": { + "packages": [ + "torch>=1.7.0", + "torchvision>=0.8.1", + "ultralytics>=8.3.0" + ], + "cpu": { + "support": true + }, + "gpu": { + "support": true + } + }, + "tags": ["detection", "coco", "torch", "yolo"], + "date_added": "2024-10-05 19:22:51" + }, + { + "base_name": "yolov11m-coco-torch", + "base_filename": "yolov11m-coco.pt", + "description": "YOLOv11-M model trained on COCO", + "source": "https://docs.ultralytics.com/models/yolov11/", + "size_bytes": 40684120, + "manager": { + "type": "fiftyone.core.models.ModelManager", + "config": { + "url": "https://github.com/ultralytics/assets/releases/download/v8.3.0/yolo11m.pt" + } + }, + "default_deployment_config_dict": { + "type": "fiftyone.utils.ultralytics.FiftyOneYOLODetectionModel", + "config": {} + }, + "requirements": { + "packages": [ + "torch>=1.7.0", + "torchvision>=0.8.1", + "ultralytics>=8.3.0" + ], + "cpu": { + "support": true + }, + "gpu": { + "support": true + } + }, + "tags": ["detection", "coco", "torch", "yolo"], + "date_added": "2024-10-05 19:22:51" + }, + { + "base_name": "yolov11l-coco-torch", + "base_filename": "yolov11l-coco.pt", + "description": "YOLOv11-L model trained on COCO", + "source": "https://docs.ultralytics.com/models/yolov11/", + "size_bytes": 51387343, + "manager": { + "type": "fiftyone.core.models.ModelManager", + "config": { + "url": "https://github.com/ultralytics/assets/releases/download/v8.3.0/yolo11l.pt" + } + }, + "default_deployment_config_dict": { + "type": "fiftyone.utils.ultralytics.FiftyOneYOLODetectionModel", + "config": {} + }, + "requirements": { + "packages": [ + "torch>=1.7.0", + "torchvision>=0.8.1", + "ultralytics>=8.3.0" + ], + "cpu": { + "support": true + }, + "gpu": { + "support": true + } + }, + "tags": ["detection", "coco", "torch", "yolo"], + "date_added": "2024-10-05 19:22:51" + }, + { + "base_name": "yolov11x-coco-torch", + "base_filename": "yolov11x-coco.pt", + "description": "YOLOv11-X model trained on COCO", + "source": "https://docs.ultralytics.com/models/yolov11/", + "size_bytes": 114636239, + "manager": { + "type": "fiftyone.core.models.ModelManager", + "config": { + "url": "https://github.com/ultralytics/assets/releases/download/v8.3.0/yolo11x.pt" + } + }, + "default_deployment_config_dict": { + "type": "fiftyone.utils.ultralytics.FiftyOneYOLODetectionModel", + "config": {} + }, + "requirements": { + "packages": [ + "torch>=1.7.0", + "torchvision>=0.8.1", + "ultralytics>=8.3.0" + ], + "cpu": { + "support": true + }, + "gpu": { + "support": true + } + }, + "tags": ["detection", "coco", "torch", "yolo"], + "date_added": "2024-10-05 19:22:51" + }, { "base_name": "rtdetr-l-coco-torch", "base_filename": "rtdetr-l-coco.pt", From e396c740c259acfbf4c5c17199b42a7ef1953a61 Mon Sep 17 00:00:00 2001 From: Jacob Marks Date: Sat, 5 Oct 2024 12:26:52 -0400 Subject: [PATCH 08/69] update syntax to match ultralytics --- fiftyone/zoo/models/manifest-torch.json | 30 ++++++++++++------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/fiftyone/zoo/models/manifest-torch.json b/fiftyone/zoo/models/manifest-torch.json index 1d748dcda8..be56017924 100644 --- a/fiftyone/zoo/models/manifest-torch.json +++ b/fiftyone/zoo/models/manifest-torch.json @@ -3261,9 +3261,9 @@ "date_added": "2024-07-01 19:22:51" }, { - "base_name": "yolov11n-coco-torch", - "base_filename": "yolov11n-coco.pt", - "description": "YOLOv11-N model trained on COCO", + "base_name": "yolo11n-coco-torch", + "base_filename": "yolo11n-coco.pt", + "description": "YOLO11-N model trained on COCO", "source": "https://docs.ultralytics.com/models/yolov11/", "size_bytes": 5613764, "manager": { @@ -3293,9 +3293,9 @@ "date_added": "2024-10-05 19:22:51" }, { - "base_name": "yolov11s-coco-torch", - "base_filename": "yolov11s-coco.pt", - "description": "YOLOv11-S model trained on COCO", + "base_name": "yolo11s-coco-torch", + "base_filename": "yolo11s-coco.pt", + "description": "YOLO11-S model trained on COCO", "source": "https://docs.ultralytics.com/models/yolov11/", "size_bytes": 19313732, "manager": { @@ -3325,9 +3325,9 @@ "date_added": "2024-10-05 19:22:51" }, { - "base_name": "yolov11m-coco-torch", - "base_filename": "yolov11m-coco.pt", - "description": "YOLOv11-M model trained on COCO", + "base_name": "yolo11m-coco-torch", + "base_filename": "yolo11m-coco.pt", + "description": "YOLO11-M model trained on COCO", "source": "https://docs.ultralytics.com/models/yolov11/", "size_bytes": 40684120, "manager": { @@ -3357,9 +3357,9 @@ "date_added": "2024-10-05 19:22:51" }, { - "base_name": "yolov11l-coco-torch", - "base_filename": "yolov11l-coco.pt", - "description": "YOLOv11-L model trained on COCO", + "base_name": "yolo11l-coco-torch", + "base_filename": "yolo11l-coco.pt", + "description": "YOLO11-L model trained on COCO", "source": "https://docs.ultralytics.com/models/yolov11/", "size_bytes": 51387343, "manager": { @@ -3389,9 +3389,9 @@ "date_added": "2024-10-05 19:22:51" }, { - "base_name": "yolov11x-coco-torch", - "base_filename": "yolov11x-coco.pt", - "description": "YOLOv11-X model trained on COCO", + "base_name": "yolo11x-coco-torch", + "base_filename": "yolo11x-coco.pt", + "description": "YOLO11-X model trained on COCO", "source": "https://docs.ultralytics.com/models/yolov11/", "size_bytes": 114636239, "manager": { From f8879f084d6275e0874b5059c4b8255cea3d7b19 Mon Sep 17 00:00:00 2001 From: Jacob Marks Date: Sat, 5 Oct 2024 12:27:07 -0400 Subject: [PATCH 09/69] add yolo11 det models to docs --- docs/source/integrations/ultralytics.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/source/integrations/ultralytics.rst b/docs/source/integrations/ultralytics.rst index 3ce0946233..79bef30e7d 100644 --- a/docs/source/integrations/ultralytics.rst +++ b/docs/source/integrations/ultralytics.rst @@ -105,6 +105,13 @@ You can directly pass Ultralytics `YOLO` or `RTDETR` detection models to # model = YOLO("yolov10l.pt) # model = YOLO("yolov10x.pt) + # YOLOv11 + # model = YOLO("yolo11n.pt) + # model = YOLO("yolo11s.pt) + # model = YOLO("yolo11m.pt) + # model = YOLO("yolo11l.pt) + # model = YOLO("yolo11x.pt) + # RTDETR # model = YOLO("rtdetr-l.pt") # model = YOLO("rtdetr-x.pt") @@ -140,6 +147,7 @@ You can also load any of these models directly from the # model_name = "yolov8m-coco-torch" # model_name = "yolov9e-coco-torch" # model_name = "yolov10s-coco-torch" + # model_name = "yolo11x-coco-torch" # model_name = "rtdetr-l-coco-torch" model = foz.load_zoo_model( From 2b3e94dc1c63d66311664268d4cd3dc4966142a6 Mon Sep 17 00:00:00 2001 From: Jacob Marks Date: Sat, 5 Oct 2024 12:36:44 -0400 Subject: [PATCH 10/69] add yolo11seg models to manifest --- fiftyone/zoo/models/manifest-torch.json | 160 ++++++++++++++++++++++++ 1 file changed, 160 insertions(+) diff --git a/fiftyone/zoo/models/manifest-torch.json b/fiftyone/zoo/models/manifest-torch.json index be56017924..79f899978b 100644 --- a/fiftyone/zoo/models/manifest-torch.json +++ b/fiftyone/zoo/models/manifest-torch.json @@ -3420,6 +3420,166 @@ "tags": ["detection", "coco", "torch", "yolo"], "date_added": "2024-10-05 19:22:51" }, + { + "base_name": "yolo11n-seg-coco-torch", + "base_filename": "yolo11n-seg-coco.pt", + "description": "YOLO11-N Segmentation model trained on COCO", + "source": "https://docs.ultralytics.com/models/yolo11/#__tabbed_1_2", + "size_bytes": 6182636, + "manager": { + "type": "fiftyone.core.models.ModelManager", + "config": { + "url": "https://github.com/ultralytics/assets/releases/download/v8.3.0/yolo11n-seg.pt" + } + }, + "default_deployment_config_dict": { + "type": "fiftyone.utils.ultralytics.FiftyOneYOLOSegmentationModel", + "config": {} + }, + "requirements": { + "packages": [ + "torch>=1.7.0", + "torchvision>=0.8.1", + "ultralytics>=8.3.0" + ], + "cpu": { + "support": true + }, + "gpu": { + "support": true + } + }, + "tags": ["segmentation", "coco", "torch", "yolo"], + "date_added": "2024-10-05 19:22:51" + }, + { + "base_name": "yolo11s-seg-coco-torch", + "base_filename": "yolo11s-seg-coco.pt", + "description": "YOLO11-S Segmentation model trained on COCO", + "source": "https://docs.ultralytics.com/models/yolo11/#__tabbed_1_2", + "size_bytes": 20669228, + "manager": { + "type": "fiftyone.core.models.ModelManager", + "config": { + "url": "https://github.com/ultralytics/assets/releases/download/v8.3.0/yolo11s-seg.pt" + } + }, + "default_deployment_config_dict": { + "type": "fiftyone.utils.ultralytics.FiftyOneYOLOSegmentationModel", + "config": {} + }, + "requirements": { + "packages": [ + "torch>=1.7.0", + "torchvision>=0.8.1", + "ultralytics>=8.3.0" + ], + "cpu": { + "support": true + }, + "gpu": { + "support": true + } + }, + "tags": ["segmentation", "coco", "torch", "yolo"], + "date_added": "2024-10-05 19:22:51" + }, + { + "base_name": "yolo11m-seg-coco-torch", + "base_filename": "yolo11m-seg-coco.pt", + "description": "YOLO11-M Segmentation model trained on COCO", + "source": "https://docs.ultralytics.com/models/yolo11/#__tabbed_1_2", + "size_bytes": 45400152, + "manager": { + "type": "fiftyone.core.models.ModelManager", + "config": { + "url": "https://github.com/ultralytics/assets/releases/download/v8.3.0/yolo11m-seg.pt" + } + }, + "default_deployment_config_dict": { + "type": "fiftyone.utils.ultralytics.FiftyOneYOLOSegmentationModel", + "config": {} + }, + "requirements": { + "packages": [ + "torch>=1.7.0", + "torchvision>=0.8.1", + "ultralytics>=8.3.0" + ], + "cpu": { + "support": true + }, + "gpu": { + "support": true + } + }, + "tags": ["segmentation", "coco", "torch", "yolo"], + "date_added": "2024-10-05 19:22:51" + }, + { + "base_name": "yolo11l-seg-coco-torch", + "base_filename": "yolo11l-seg-coco.pt", + "description": "YOLO11-L Segmentation model trained on COCO", + "source": "https://docs.ultralytics.com/models/yolo11/#__tabbed_1_2", + "size_bytes": 56096965, + "manager": { + "type": "fiftyone.core.models.ModelManager", + "config": { + "url": "https://github.com/ultralytics/assets/releases/download/v8.3.0/yolo11l-seg.pt" + } + }, + "default_deployment_config_dict": { + "type": "fiftyone.utils.ultralytics.FiftyOneYOLOSegmentationModel", + "config": {} + }, + "requirements": { + "packages": [ + "torch>=1.7.0", + "torchvision>=0.8.1", + "ultralytics>=8.3.0" + ], + "cpu": { + "support": true + }, + "gpu": { + "support": true + } + }, + "tags": ["segmentation", "coco", "torch", "yolo"], + "date_added": "2024-10-05 19:22:51" + }, + { + "base_name": "yolo11x-seg-coco-torch", + "base_filename": "yolo11x-seg-coco.pt", + "description": "YOLO11-X Segmentation model trained on COCO", + "source": "https://docs.ultralytics.com/models/yolo11/#__tabbed_1_2", + "size_bytes": 125090821, + "manager": { + "type": "fiftyone.core.models.ModelManager", + "config": { + "url": "https://github.com/ultralytics/assets/releases/download/v8.3.0/yolo11x-seg.pt" + } + }, + "default_deployment_config_dict": { + "type": "fiftyone.utils.ultralytics.FiftyOneYOLOSegmentationModel", + "config": {} + }, + "requirements": { + "packages": [ + "torch>=1.7.0", + "torchvision>=0.8.1", + "ultralytics>=8.3.0" + ], + "cpu": { + "support": true + }, + "gpu": { + "support": true + } + }, + "tags": ["segmentation", "coco", "torch", "yolo"], + "date_added": "2024-10-05 19:22:51" + }, { "base_name": "rtdetr-l-coco-torch", "base_filename": "rtdetr-l-coco.pt", From ba7e00f9f012b9ad249abd1d4a5672f3f91cc115 Mon Sep 17 00:00:00 2001 From: Jacob Marks Date: Sat, 5 Oct 2024 12:39:14 -0400 Subject: [PATCH 11/69] add yolo11seg models to docs --- docs/source/integrations/ultralytics.rst | 27 ++++++++++++++++++------ 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/docs/source/integrations/ultralytics.rst b/docs/source/integrations/ultralytics.rst index 79bef30e7d..dd01ddd16e 100644 --- a/docs/source/integrations/ultralytics.rst +++ b/docs/source/integrations/ultralytics.rst @@ -190,6 +190,11 @@ You can directly pass Ultralytics YOLO segmentation models to # model = YOLO("yolov8l-seg.pt") # model = YOLO("yolov8x-seg.pt") + # model = YOLO("yolo11s-seg.pt") + # model = YOLO("yolo11m-seg.pt") + # model = YOLO("yolo11l-seg.pt") + # model = YOLO("yolo11x-seg.pt") + dataset.apply_model(model, label_field="instances") session = fo.launch_app(dataset) @@ -215,19 +220,27 @@ manually convert Ultralytics predictions into the desired :align: center -You can also load YOLOv8 and YOLOv9 segmentation models from the +You can also load YOLOv8, YOLOv9, and YOLO11 segmentation models from the :ref:`FiftyOne Model Zoo `: .. code-block:: python :linenos: - model_name = "yolov9c-seg-coco-torch" - # model_name = "yolov9e-seg-coco-torch" - # model_name = "yolov8x-seg-coco-torch" - # model_name = "yolov8l-seg-coco-torch" - # model_name = "yolov8m-seg-coco-torch" + model_name = "yolov8n-seg-coco-torch" # model_name = "yolov8s-seg-coco-torch" - # model_name = "yolov8n-seg-coco-torch" + # model_name = "yolov8m-seg-coco-torch" + # model_name = "yolov8l-seg-coco-torch" + # model_name = "yolov8x-seg-coco-torch" + + # model_name = "yolov9c-seg-coco-torch" + # model_name = "yolov9e-seg-coco-torch" + + # model_name = "yolo11n-seg-coco-torch" + # model_name = "yolo11s-seg-coco-torch" + # model_name = "yolo11m-seg-coco-torch" + # model_name = "yolo11l-seg-coco-torch" + # model_name = "yolo11x-seg-coco-torch" + model = foz.load_zoo_model(model_name, label_field="yolo_seg") From 7f4f7a5d09d734ad8f62adce20c93d92f89cf6a6 Mon Sep 17 00:00:00 2001 From: brimoor Date: Mon, 7 Oct 2024 12:01:28 +0200 Subject: [PATCH 12/69] thanks coderabbit --- fiftyone/utils/iou.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/fiftyone/utils/iou.py b/fiftyone/utils/iou.py index 3d838c5f22..65d32229c0 100644 --- a/fiftyone/utils/iou.py +++ b/fiftyone/utils/iou.py @@ -87,8 +87,9 @@ def compute_ious( error will default to an IoU of 0 Returns: - a ``num_preds x num_gts`` array of IoUs when ``sparse=False`, or a dict - of the form ``d[pred.id] = [(gt.id, iou), ...]`` when ``sparse=True`` + a ``num_preds x num_gts`` array of IoUs when ``sparse=False``, or a + dict of the form ``d[pred.id] = [(gt.id, iou), ...]`` when + ``sparse=True`` """ if not preds or not gts: if sparse: @@ -172,7 +173,7 @@ def compute_segment_ious(preds, gts, sparse=False): than a full matrix Returns: - a ``num_preds x num_gts`` array of segment IoUs when ``sparse=False`, + a ``num_preds x num_gts`` array of segment IoUs when ``sparse=False``, or a dict of the form ``d[pred.id] = [(gt.id, iou), ...]`` when ``sparse=True`` """ @@ -548,7 +549,7 @@ def _get_detection_box(det, dimension=None): zmax = np.max(box.vertices[:, 2]) result = (xmin, xmax, ymin, ymax, zmin, zmax) else: - raise Exception(f"dimension should be 2 or 3, but got {dimension}") + raise ValueError(f"dimension should be 2 or 3, but got {dimension}") return result From 6f7485acf791bceb65937c3d97ca34948ac6c66d Mon Sep 17 00:00:00 2001 From: Br2850 Date: Fri, 11 Oct 2024 19:44:30 -0600 Subject: [PATCH 13/69] track panel closes --- app/packages/operators/src/CustomPanel.tsx | 7 ++++++- app/packages/operators/src/useCustomPanelHooks.ts | 6 ++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/app/packages/operators/src/CustomPanel.tsx b/app/packages/operators/src/CustomPanel.tsx index cf9f82a5b3..dcbd671fce 100644 --- a/app/packages/operators/src/CustomPanel.tsx +++ b/app/packages/operators/src/CustomPanel.tsx @@ -12,7 +12,11 @@ import OperatorIO from "./OperatorIO"; import { PANEL_LOAD_TIMEOUT } from "./constants"; import { useActivePanelEventsCount } from "./hooks"; import { Property } from "./types"; -import { CustomPanelProps, useCustomPanelHooks } from "./useCustomPanelHooks"; +import { + CustomPanelProps, + useCustomPanelHooks, + trackPanelClose, +} from "./useCustomPanelHooks"; export function CustomPanel(props: CustomPanelProps) { const { panelId, dimensions, panelName, panelLabel } = props; @@ -33,6 +37,7 @@ export function CustomPanel(props: CustomPanelProps) { useEffect(() => { setPanelCloseEffect(() => { clearUseKeyStores(panelId); + trackPanelClose(panelName); }); }, []); diff --git a/app/packages/operators/src/useCustomPanelHooks.ts b/app/packages/operators/src/useCustomPanelHooks.ts index afbaf851dd..95ae2a9cd1 100644 --- a/app/packages/operators/src/useCustomPanelHooks.ts +++ b/app/packages/operators/src/useCustomPanelHooks.ts @@ -1,3 +1,4 @@ +import { useTrackEvent } from "@fiftyone/analytics"; import { debounce, merge } from "lodash"; import { useCallback, useEffect, useMemo } from "react"; @@ -223,3 +224,8 @@ function getPanelViewData(panelState) { const data = panelState?.data; return merge({}, { ...state }, { ...data }); } + +export function trackPanelClose(panelName) { + const trackEvent = useTrackEvent(); + return trackEvent("close_panel", { panel_name: panelName }); +} From 8c279f3008af05bcbc17afb8013758ae13d90f22 Mon Sep 17 00:00:00 2001 From: Sashank Aryal Date: Mon, 14 Oct 2024 21:36:21 +0545 Subject: [PATCH 14/69] fix shortcutToHelpItems --- app/packages/core/src/components/Modal/utils.ts | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/app/packages/core/src/components/Modal/utils.ts b/app/packages/core/src/components/Modal/utils.ts index 2461570f9a..e20a1ec6a6 100644 --- a/app/packages/core/src/components/Modal/utils.ts +++ b/app/packages/core/src/components/Modal/utils.ts @@ -1,7 +1,13 @@ -export function shortcutToHelpItems(SHORTCUTS) { - const result = {}; - for (const k of SHORTCUTS) { - result[SHORTCUTS[k].shortcut] = SHORTCUTS[k]; +interface ShortcutItem { + shortcut: string; +} + +type Shortcuts = { [key: string]: ShortcutItem }; + +export function shortcutToHelpItems(SHORTCUTS: Shortcuts) { + const uniqueItems = {}; + for (const item of Object.values(SHORTCUTS)) { + uniqueItems[item.shortcut] = item; } - return Object.values(result); + return Object.values(uniqueItems); } From d112a648ae16ba3698df4dfd94bb3c654f9dc502 Mon Sep 17 00:00:00 2001 From: Sashank Aryal Date: Mon, 14 Oct 2024 22:47:09 +0545 Subject: [PATCH 15/69] add < > shortcuts in help modal --- .../looker/src/elements/common/actions.ts | 24 +++++++++++++++++++ app/packages/looker/src/lookers/image.ts | 16 ++++++++++++- app/packages/looker/src/state.ts | 1 + .../state/src/hooks/useCreateLooker.ts | 3 +++ 4 files changed, 43 insertions(+), 1 deletion(-) diff --git a/app/packages/looker/src/elements/common/actions.ts b/app/packages/looker/src/elements/common/actions.ts index 5775dda7f7..a8cdbffb84 100644 --- a/app/packages/looker/src/elements/common/actions.ts +++ b/app/packages/looker/src/elements/common/actions.ts @@ -405,6 +405,17 @@ export const nextFrame: Control = { }, }; +export const nextFrameNoOpControl: Control = { + title: "Next frame", + eventKeys: [".", ">"], + shortcut: ">", + detail: "Seek to the next frame", + alwaysHandle: true, + action: () => { + // no-op here, supposed to be implemented elsewhere + }, +}; + export const previousFrame: Control = { title: "Previous frame", eventKeys: [",", "<"], @@ -436,6 +447,17 @@ export const previousFrame: Control = { }, }; +export const previousFrameNoOpControl: Control = { + title: "Previous frame", + eventKeys: [",", "<"], + shortcut: "<", + detail: "Seek to the previous frame", + alwaysHandle: true, + action: () => { + // no-op here, supposed to be implemented elsewhere + }, +}; + export const playPause: Control = { title: "Play / pause", shortcut: "Space", @@ -662,6 +684,8 @@ const VIDEO = { const IMAVID = { ...COMMON, escape: videoEscape, + previousFrame: previousFrameNoOpControl, + nextFrame: nextFrameNoOpControl, }; export const VIDEO_SHORTCUTS = readActions(VIDEO); diff --git a/app/packages/looker/src/lookers/image.ts b/app/packages/looker/src/lookers/image.ts index 5e20d9670a..98780c519c 100644 --- a/app/packages/looker/src/lookers/image.ts +++ b/app/packages/looker/src/lookers/image.ts @@ -5,6 +5,10 @@ import { DEFAULT_IMAGE_OPTIONS, ImageState } from "../state"; import { AbstractLooker } from "./abstract"; import { LookerUtils } from "./shared"; +import { + nextFrameNoOpControl, + previousFrameNoOpControl, +} from "../elements/common/actions"; import { zoomToContent } from "../zoom"; export class ImageLooker extends AbstractLooker { @@ -21,11 +25,21 @@ export class ImageLooker extends AbstractLooker { ...options, }; + // if in dynamic groups mode, add < > shortcuts, too + let shortcuts = { ...COMMON_SHORTCUTS }; + if (config.isDynamicGroup) { + shortcuts = { + ...COMMON_SHORTCUTS, + previousFrameNoOpControl, + nextFrameNoOpControl, + }; + } + return { ...this.getInitialBaseState(), config: { ...config }, options, - SHORTCUTS: COMMON_SHORTCUTS, + SHORTCUTS: shortcuts, }; } diff --git a/app/packages/looker/src/state.ts b/app/packages/looker/src/state.ts index 84281f154a..e31eb20652 100644 --- a/app/packages/looker/src/state.ts +++ b/app/packages/looker/src/state.ts @@ -203,6 +203,7 @@ export interface BaseConfig { sampleId: string; symbol: symbol; fieldSchema: Schema; + isDynamicGroup: boolean; view: Stage[]; dataset: string; group?: { diff --git a/app/packages/state/src/hooks/useCreateLooker.ts b/app/packages/state/src/hooks/useCreateLooker.ts index 1fc1b748e3..1150346033 100644 --- a/app/packages/state/src/hooks/useCreateLooker.ts +++ b/app/packages/state/src/hooks/useCreateLooker.ts @@ -61,6 +61,8 @@ export default >( dynamicGroupAtoms.shouldRenderImaVidLooker(isModal) ); + const isDynamicGroup = useRecoilValue(dynamicGroupAtoms.isDynamicGroup); + // callback to get the latest promise inside another recoil callback // gets around the limitation of the fact that snapshot inside callback refs to the committed state at the time const getPromise = useRecoilCallback( @@ -128,6 +130,7 @@ export default >( sources: urls, frameNumber: create === FrameLooker ? frameNumber : undefined, frameRate, + isDynamicGroup, sampleId: sample._id, support: isClip ? sample.support : undefined, dataset, From b330944aee7fe804d33d63804335b519960d0ccb Mon Sep 17 00:00:00 2001 From: Benjamin Kane Date: Mon, 14 Oct 2024 14:29:07 -0400 Subject: [PATCH 16/69] add test for shortcuts --- app/packages/core/src/components/Modal/utils.test.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 app/packages/core/src/components/Modal/utils.test.ts diff --git a/app/packages/core/src/components/Modal/utils.test.ts b/app/packages/core/src/components/Modal/utils.test.ts new file mode 100644 index 0000000000..d764324867 --- /dev/null +++ b/app/packages/core/src/components/Modal/utils.test.ts @@ -0,0 +1,12 @@ +import { describe, expect, it } from "vitest"; +import { shortcutToHelpItems } from "./utils"; + +describe("shortcut processing test", () => { + it("parses unique shortcuts", () => { + const results = shortcutToHelpItems({ + one: { shortcut: "test" }, + two: { shortcut: "test" }, + }); + expect(results).toStrictEqual([{ shortcut: "test" }]); + }); +}); From 19f667340522efe40aaebd9ee3299b3632fba967 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 15 Oct 2024 16:34:16 -0400 Subject: [PATCH 17/69] Bump starlette from 0.36.2 to 0.40.0 in /requirements (#4929) Bumps [starlette](https://github.com/encode/starlette) from 0.36.2 to 0.40.0. - [Release notes](https://github.com/encode/starlette/releases) - [Changelog](https://github.com/encode/starlette/blob/master/docs/release-notes.md) - [Commits](https://github.com/encode/starlette/compare/0.36.2...0.40.0) --- updated-dependencies: - dependency-name: starlette dependency-type: direct:production ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- requirements/common.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/common.txt b/requirements/common.txt index b0a2a3b338..5ce296d2ac 100644 --- a/requirements/common.txt +++ b/requirements/common.txt @@ -27,7 +27,7 @@ scikit-image>=0.16.2 setuptools>=45.2.0,<71 sseclient-py>=1.7.2 sse-starlette>=0.10.3 -starlette==0.36.2 +starlette==0.40.0 strawberry-graphql==0.243.0 tabulate==0.8.10 xmltodict==0.12.0 From 92f7e1f6f8f91ab1efb9af34fc3327033e79431d Mon Sep 17 00:00:00 2001 From: Minh-Tue Vo Date: Tue, 15 Oct 2024 14:57:00 -0700 Subject: [PATCH 18/69] Lightning mode refactor and disable/enable qp mode toggle (#4928) * Refactored Lightning Mode to QP mode * Added condition for checking to deploy vars * Updated tests --------- Co-authored-by: minhtuevo --- .../core/src/components/Actions/Options.tsx | 19 +++++++++++-------- .../components/FieldLabelAndInfo/index.tsx | 4 ++-- .../Entries/FilterablePathEntry/Arrow.tsx | 4 ++-- .../Entries/FilterablePathEntry/Tune.tsx | 4 ++-- app/packages/core/src/utils/links.ts | 2 +- app/packages/state/src/recoil/lightning.ts | 10 ++++++++++ .../oss/poms/action-row/display-options.ts | 2 +- 7 files changed, 29 insertions(+), 16 deletions(-) diff --git a/app/packages/core/src/components/Actions/Options.tsx b/app/packages/core/src/components/Actions/Options.tsx index b92a509e6b..36dc1de288 100644 --- a/app/packages/core/src/components/Actions/Options.tsx +++ b/app/packages/core/src/components/Actions/Options.tsx @@ -17,7 +17,7 @@ import { useResetRecoilState, useSetRecoilState, } from "recoil"; -import { LIGHTNING_MODE, SIDEBAR_MODE } from "../../utils/links"; +import { QP_MODE, SIDEBAR_MODE } from "../../utils/links"; import Checkbox from "../Common/Checkbox"; import RadioGroup from "../Common/RadioGroup"; import { Button } from "../utils"; @@ -216,19 +216,22 @@ const DynamicGroupsViewMode = ({ modal }: { modal: boolean }) => { ); }; -const Lightning = () => { +const QueryPerformance = () => { const [threshold, setThreshold] = useRecoilState(fos.lightningThreshold); const config = useRecoilValue(fos.lightningThresholdConfig); const reset = useResetRecoilState(fos.lightningThreshold); const count = useRecoilValue(fos.datasetSampleCount); const theme = useTheme(); + const enableQueryPerformanceConfig = useRecoilValue(fos.enableQueryPerformanceConfig); + const defaultQueryPerformanceConfig = useRecoilValue(fos.defaultQueryPerformanceConfig); + const enableQpMode = enableQueryPerformanceConfig && defaultQueryPerformanceConfig; - return ( + if (enableQpMode) return ( <> { svgStyles={{ height: "1rem", marginTop: 7.5 }} /> ({ text: value, title: value, - dataCy: `lightning-mode-${value}`, + dataCy: `qp-mode-${value}`, onClick: () => setThreshold(value === "disable" ? null : config ?? count), }))} @@ -372,7 +375,7 @@ const Options = ({ modal, anchorRef }: OptionsProps) => { {isGroup && !isDynamicGroup && } - {!view?.length && } + {!view?.length && } {!modal && } diff --git a/app/packages/core/src/components/FieldLabelAndInfo/index.tsx b/app/packages/core/src/components/FieldLabelAndInfo/index.tsx index 08646a1a12..348863fbd0 100644 --- a/app/packages/core/src/components/FieldLabelAndInfo/index.tsx +++ b/app/packages/core/src/components/FieldLabelAndInfo/index.tsx @@ -20,7 +20,7 @@ import { } from "recoil"; import styled from "styled-components"; import { ExternalLink } from "../../utils/generic"; -import { LIGHTNING_MODE } from "../../utils/links"; +import { QP_MODE } from "../../utils/links"; import { activeColorEntry } from "../ColorModal/state"; const selectedFieldInfo = atom({ @@ -345,7 +345,7 @@ const Lightning: React.FunctionComponent = ({ color, path }) => { Lightning indexed diff --git a/app/packages/core/src/components/Sidebar/Entries/FilterablePathEntry/Arrow.tsx b/app/packages/core/src/components/Sidebar/Entries/FilterablePathEntry/Arrow.tsx index b635a7c7f4..2d385d314e 100644 --- a/app/packages/core/src/components/Sidebar/Entries/FilterablePathEntry/Arrow.tsx +++ b/app/packages/core/src/components/Sidebar/Entries/FilterablePathEntry/Arrow.tsx @@ -7,7 +7,7 @@ import { RecoilState, useRecoilState } from "recoil"; import { useTheme } from "styled-components"; import { FRAME_FILTERING_DISABLED, - LIGHTNING_MODE, + QP_MODE, } from "../../../../utils/links"; import DisabledReason from "./DisabledReason"; @@ -60,7 +60,7 @@ export default ({ if (unindexed && !unlocked) { return ( } + text={} placement="top-center" > {arrow} diff --git a/app/packages/core/src/components/Sidebar/Entries/FilterablePathEntry/Tune.tsx b/app/packages/core/src/components/Sidebar/Entries/FilterablePathEntry/Tune.tsx index 66c3ac3db1..e1bba84dc8 100644 --- a/app/packages/core/src/components/Sidebar/Entries/FilterablePathEntry/Tune.tsx +++ b/app/packages/core/src/components/Sidebar/Entries/FilterablePathEntry/Tune.tsx @@ -3,7 +3,7 @@ import { Tune } from "@mui/icons-material"; import React from "react"; import type { RecoilState } from "recoil"; import { useSetRecoilState } from "recoil"; -import { LIGHTNING_MODE } from "../../../../utils/links"; +import { QP_MODE } from "../../../../utils/links"; import DisabledReason from "./DisabledReason"; export default ({ @@ -54,7 +54,7 @@ export default ({ if (disabled) { return ( } + text={} placement="top-center" > {children} diff --git a/app/packages/core/src/utils/links.ts b/app/packages/core/src/utils/links.ts index f3087713e8..68490756a6 100644 --- a/app/packages/core/src/utils/links.ts +++ b/app/packages/core/src/utils/links.ts @@ -13,7 +13,7 @@ export const FIELD_METADATA = export const FRAME_FILTERING_DISABLED = "https://docs.voxel51.com/user_guide/using_datasets.html#disable-frame-filtering"; -export const LIGHTNING_MODE = +export const QP_MODE = "https://docs.voxel51.com/user_guide/app.html#lightning-mode"; export const NAME_COLORSCALE = "https://plotly.com/python/colorscales/"; diff --git a/app/packages/state/src/recoil/lightning.ts b/app/packages/state/src/recoil/lightning.ts index e206fcde47..0c0ad7950a 100644 --- a/app/packages/state/src/recoil/lightning.ts +++ b/app/packages/state/src/recoil/lightning.ts @@ -200,6 +200,16 @@ export const lightningThresholdConfig = selector({ get: ({ get }) => get(config).lightningThreshold, }); +export const enableQueryPerformanceConfig = selector({ + key: "enableQueryPerformanceConfig", + get: ({ get }) => get(config).enableQueryPerformance, +}); + +export const defaultQueryPerformanceConfig = selector({ + key: "defaultQueryPerformanceConfig", + get: ({ get }) => get(config).defaultQueryPerformance, +}); + const lightningThresholdAtom = atomFamily({ key: "lightningThresholdAtom", default: undefined, diff --git a/e2e-pw/src/oss/poms/action-row/display-options.ts b/e2e-pw/src/oss/poms/action-row/display-options.ts index 3f229e2df7..57cef8d9d9 100644 --- a/e2e-pw/src/oss/poms/action-row/display-options.ts +++ b/e2e-pw/src/oss/poms/action-row/display-options.ts @@ -13,7 +13,7 @@ export class DisplayOptionsPom { } async setLightningMode(mode: LightningMode) { - const selector = this.page.getByTestId(`lightning-mode-${mode}`); + const selector = this.page.getByTestId(`qp-mode-${mode}`); return selector.click(); } From e980ee4e060ee4e96d0ef8458ef70a0f8d307e70 Mon Sep 17 00:00:00 2001 From: brimoor Date: Wed, 16 Oct 2024 08:44:03 -0400 Subject: [PATCH 19/69] clarifying model vs source metadata for remote zoo models --- docs/source/model_zoo/remote.rst | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/docs/source/model_zoo/remote.rst b/docs/source/model_zoo/remote.rst index bfb6242fb4..fe513589d0 100644 --- a/docs/source/model_zoo/remote.rst +++ b/docs/source/model_zoo/remote.rst @@ -231,10 +231,6 @@ model(s) that it contains: +----------------------------------+-----------+-------------------------------------------------------------------------------------------+ | Field | Required? | Description | +==================================+===========+===========================================================================================+ - | `name` | | A name for the remote model source | - +----------------------------------+-----------+-------------------------------------------------------------------------------------------+ - | `url` | | The URL of the remote model source | - +----------------------------------+-----------+-------------------------------------------------------------------------------------------+ | `base_name` | **yes** | The base name of the model (no version info) | +----------------------------------+-----------+-------------------------------------------------------------------------------------------+ | `base_filename` | | The base filename or directory of the model (no version info), if applicable. | @@ -279,6 +275,19 @@ model(s) that it contains: | | | must be provided | +----------------------------------+-----------+-------------------------------------------------------------------------------------------+ +It can also provide optional metadata about the remote source itself: + +.. table:: + :widths: 20,10,70 + + +----------------------------------+-----------+-------------------------------------------------------------------------------------------+ + | Field | Required? | Description | + +==================================+===========+===========================================================================================+ + | `name` | | A name for the remote model source | + +----------------------------------+-----------+-------------------------------------------------------------------------------------------+ + | `url` | | The URL of the remote model source | + +----------------------------------+-----------+-------------------------------------------------------------------------------------------+ + Here's an exaxmple model manifest file that declares a single model: .. code-block:: json From c9b2a93e1ec2a85faa5ba4726241118e76433efa Mon Sep 17 00:00:00 2001 From: Sashank Aryal Date: Wed, 16 Oct 2024 20:36:38 +0545 Subject: [PATCH 20/69] add support for ignoring modifier keys in useHotKey --- .../looker-3d/src/hooks/use-hot-key.ts | 28 ++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/app/packages/looker-3d/src/hooks/use-hot-key.ts b/app/packages/looker-3d/src/hooks/use-hot-key.ts index ada37f5ab1..b76c5532ac 100644 --- a/app/packages/looker-3d/src/hooks/use-hot-key.ts +++ b/app/packages/looker-3d/src/hooks/use-hot-key.ts @@ -14,24 +14,44 @@ export const useHotkey = ( snapshot: recoil.Snapshot; }) => void, deps: readonly unknown[] = [], - useTransaction = true + props: { + useTransaction?: boolean; + ignoreModifiers?: boolean; + } = { useTransaction: true, ignoreModifiers: true } ) => { - const cbAsRecoilTransaction = useTransaction + if (typeof props.useTransaction === "undefined") { + props.useTransaction = true; + } + if (typeof props.ignoreModifiers === "undefined") { + props.ignoreModifiers = true; + } + + const { useTransaction, ignoreModifiers } = props; + + const decoratedCb = useTransaction ? useRecoilTransaction_UNSTABLE((ctx) => () => cb(ctx), deps) : recoil.useRecoilCallback((ctx) => () => cb(ctx), deps); const handle = useCallback( (e: KeyboardEventUnionType) => { + // ignore if modifier keys are pressed + if ( + ignoreModifiers && + (e.ctrlKey || e.metaKey || e.altKey || e.shiftKey) + ) { + return; + } + const active = document.activeElement; if (active?.tagName === "INPUT") { return; } if (e.code === keyCode) { - cbAsRecoilTransaction(); + decoratedCb(); } }, - [cbAsRecoilTransaction, keyCode] + [decoratedCb, keyCode] ); useEffect(() => { From d3532b6d10df961b3b4e2e487c16a3ecbe0f3bc2 Mon Sep 17 00:00:00 2001 From: Sashank Aryal Date: Wed, 16 Oct 2024 20:37:50 +0545 Subject: [PATCH 21/69] poll multiple times before finalizing bounding box of scene --- app/packages/looker-3d/src/Looker3d.tsx | 4 +- app/packages/looker-3d/src/MediaTypePcd.tsx | 8 +++- .../looker-3d/src/action-bar/ViewFo3d.tsx | 2 +- .../looker-3d/src/action-bar/index.tsx | 2 +- .../looker-3d/src/hooks/use-bounds.ts | 46 +++++++++++++++---- 5 files changed, 49 insertions(+), 13 deletions(-) diff --git a/app/packages/looker-3d/src/Looker3d.tsx b/app/packages/looker-3d/src/Looker3d.tsx index 3a5d696108..eab3537838 100644 --- a/app/packages/looker-3d/src/Looker3d.tsx +++ b/app/packages/looker-3d/src/Looker3d.tsx @@ -125,7 +125,9 @@ export const Looker3d = () => { set(fos.modalSelector, null); }, [sampleMap, isHovering], - false + { + useTransaction: false, + } ); const clear = useCallback(() => { diff --git a/app/packages/looker-3d/src/MediaTypePcd.tsx b/app/packages/looker-3d/src/MediaTypePcd.tsx index 380553adf5..710f2e9fe3 100644 --- a/app/packages/looker-3d/src/MediaTypePcd.tsx +++ b/app/packages/looker-3d/src/MediaTypePcd.tsx @@ -177,8 +177,12 @@ export const MediaTypePcdComponent = () => { const setCurrentAction = useSetRecoilState(currentActionAtom); - useHotkey("KeyT", () => onChangeView("top"), [onChangeView], false); - useHotkey("KeyE", () => onChangeView("pov"), [onChangeView], false); + useHotkey("KeyT", () => onChangeView("top"), [onChangeView], { + useTransaction: false, + }); + useHotkey("KeyE", () => onChangeView("pov"), [onChangeView], { + useTransaction: false, + }); useEffect(() => { const currentCameraPosition = cameraRef.current?.position; diff --git a/app/packages/looker-3d/src/action-bar/ViewFo3d.tsx b/app/packages/looker-3d/src/action-bar/ViewFo3d.tsx index 9980592c0a..8244f3fb65 100644 --- a/app/packages/looker-3d/src/action-bar/ViewFo3d.tsx +++ b/app/packages/looker-3d/src/action-bar/ViewFo3d.tsx @@ -40,7 +40,7 @@ export const ViewFo3d = (props: { return () => {}; }, [jsonPanel], - false + { useTransaction: false } ); fos.useOutsideClick(buttonRef, () => { diff --git a/app/packages/looker-3d/src/action-bar/index.tsx b/app/packages/looker-3d/src/action-bar/index.tsx index 231ed7204d..d0481fef6e 100644 --- a/app/packages/looker-3d/src/action-bar/index.tsx +++ b/app/packages/looker-3d/src/action-bar/index.tsx @@ -55,7 +55,7 @@ export const ActionBar = ({ jsonPanel.toggle(sampleForJsonView); }, [sampleForJsonView], - false + { useTransaction: false } ); const componentsToRender = useMemo(() => { diff --git a/app/packages/looker-3d/src/hooks/use-bounds.ts b/app/packages/looker-3d/src/hooks/use-bounds.ts index 45c9291e30..fc28a93095 100644 --- a/app/packages/looker-3d/src/hooks/use-bounds.ts +++ b/app/packages/looker-3d/src/hooks/use-bounds.ts @@ -1,16 +1,16 @@ -import { useLayoutEffect, useState } from "react"; +import { useLayoutEffect, useRef, useState } from "react"; import { Box3, type Group } from "three"; const BOUNDING_BOX_POLLING_INTERVAL = 50; +const UNCHANGED_COUNT_THRESHOLD = 6; /** - * Calaculates the bounding box of the object with given ref. + * Calculates the bounding box of the object with the given ref. * - * - * @param objectRef ref to the object - * @param predicate optional predicate to check before calculating the bounding box. + * @param objectRef - Ref to the object + * @param predicate - Optional predicate to check before calculating the bounding box. * IMPORTANT: Make sure this predicate is memoized using useCallback - * @returns bounding box of the object + * @returns Bounding box of the object */ export const useFo3dBounds = ( objectRef: React.RefObject, @@ -18,29 +18,59 @@ export const useFo3dBounds = ( ) => { const [sceneBoundingBox, setSceneBoundingBox] = useState(null); + const unchangedCount = useRef(0); + const previousBox = useRef(null); + useLayoutEffect(() => { if (predicate && !predicate()) { return; } + // flag to prevent state updates on unmounted components + let isMounted = true; + + const boxesAreEqual = (box1: Box3, box2: Box3) => { + return box1.min.equals(box2.min) && box1.max.equals(box2.max); + }; + const getBoundingBox = () => { + if (!isMounted) return; + if (!objectRef.current) { setTimeout(getBoundingBox, BOUNDING_BOX_POLLING_INTERVAL); return; } const box = new Box3().setFromObject(objectRef.current); + if (Math.abs(box.max?.x) === Number.POSITIVE_INFINITY) { setTimeout(getBoundingBox, BOUNDING_BOX_POLLING_INTERVAL); + return; + } + + if (previousBox.current && boxesAreEqual(box, previousBox.current)) { + unchangedCount.current += 1; } else { + unchangedCount.current = 1; + } + + previousBox.current = box; + + if (unchangedCount.current >= UNCHANGED_COUNT_THRESHOLD) { setSceneBoundingBox(box); + } else { + setTimeout(getBoundingBox, BOUNDING_BOX_POLLING_INTERVAL); } }; - // this is a hack, yet to find a robust way to know when the scene is done loading + // this is a hack, yet to find a better way than polling to know when the scene is done loading // callbacks in loaders are not reliable - // check every 50ms for scene's bounding box setTimeout(getBoundingBox, BOUNDING_BOX_POLLING_INTERVAL); + + // cleanup function to prevent memory leaks + return () => { + isMounted = false; + }; }, [objectRef, predicate]); return sceneBoundingBox; From 15ce9a2f342813c1d11d6ab9e8bc0322afa47dc2 Mon Sep 17 00:00:00 2001 From: Sashank Aryal Date: Wed, 16 Oct 2024 20:38:57 +0545 Subject: [PATCH 22/69] zoom and setlookout on z press --- .../looker-3d/src/fo3d/MediaTypeFo3d.tsx | 96 ++++++++++++++++++- 1 file changed, 94 insertions(+), 2 deletions(-) diff --git a/app/packages/looker-3d/src/fo3d/MediaTypeFo3d.tsx b/app/packages/looker-3d/src/fo3d/MediaTypeFo3d.tsx index 9aed454116..331928bea4 100644 --- a/app/packages/looker-3d/src/fo3d/MediaTypeFo3d.tsx +++ b/app/packages/looker-3d/src/fo3d/MediaTypeFo3d.tsx @@ -13,7 +13,7 @@ import { useState, } from "react"; import { useRecoilCallback, useRecoilValue } from "recoil"; -import type * as THREE from "three"; +import * as THREE from "three"; import { type PerspectiveCamera, Vector3 } from "three"; import { SpinningCube } from "../SpinningCube"; import { StatusBar, StatusTunnel } from "../StatusBar"; @@ -376,6 +376,98 @@ export const MediaTypeFo3dComponent = () => { [onChangeView] ); + // zoom to selected labels and use them as the new lookAt + useHotkey( + "KeyZ", + async ({ snapshot }) => { + const currentSelectedLabels = await snapshot.getPromise( + fos.selectedLabels + ); + const labelsFieldNames = currentSelectedLabels.map((l) => l.field); + + const labelBoundingBoxes = []; + for (const labelKey of labelsFieldNames) { + const thisLabelDimension = sample.sample[labelKey].dimensions as [ + number, + number, + number + ]; + const thisLabelLocation = sample.sample[labelKey].location as [ + number, + number, + number + ]; + + const thisLabelBoundingBox = new THREE.Box3(); + thisLabelBoundingBox.setFromCenterAndSize( + new THREE.Vector3(...thisLabelLocation), + new THREE.Vector3(...thisLabelDimension) + ); + + labelBoundingBoxes.push(thisLabelBoundingBox); + } + + const unionBoundingBox = labelBoundingBoxes[0].clone(); + + for (let i = 1; i < labelBoundingBoxes.length; i++) { + unionBoundingBox.union(labelBoundingBoxes[i]); + } + + const unionBoundingBoxCenter = unionBoundingBox.getCenter( + new THREE.Vector3() + ); + const unionBoundingBoxSize = unionBoundingBox.getSize( + new THREE.Vector3() + ); + + const maxSize = Math.max( + unionBoundingBoxSize.x, + unionBoundingBoxSize.y, + unionBoundingBoxSize.z + ); + + const newCameraPosition = new THREE.Vector3(); + + if (upVector.y === 1) { + newCameraPosition.set( + unionBoundingBoxCenter.x, + // times 3 (arbitrary) to make sure the camera is not inside the bounding box + unionBoundingBoxCenter.y + maxSize * 3, + unionBoundingBoxCenter.z + ); + } else if (upVector.x === 1) { + newCameraPosition.set( + // times 3 (arbitrary) to make sure the camera is not inside the bounding box + unionBoundingBoxCenter.x + maxSize * 2, + unionBoundingBoxCenter.y, + unionBoundingBoxCenter.z + ); + } else { + // assume z-up + newCameraPosition.set( + unionBoundingBoxCenter.x, + unionBoundingBoxCenter.y, + // times 3 (arbitrary) to make sure the camera is not inside the bounding box + unionBoundingBoxCenter.z + maxSize * 2 + ); + } + + await cameraControlsRef.current.setLookAt( + newCameraPosition.x, + newCameraPosition.y, + newCameraPosition.z, + unionBoundingBoxCenter.x, + unionBoundingBoxCenter.y, + unionBoundingBoxCenter.z, + true + ); + }, + [sample, upVector], + { + useTransaction: false, + } + ); + useEffect(() => { if (!cameraControlsRef.current) { return; @@ -418,7 +510,7 @@ export const MediaTypeFo3dComponent = () => { > - + From da5f01d2c7b5d676f33f566c4fd67f4656e27875 Mon Sep 17 00:00:00 2001 From: Sashank Aryal Date: Wed, 16 Oct 2024 21:21:37 +0545 Subject: [PATCH 23/69] add test for use fo3d bounds --- .../looker-3d/src/hooks/use-bounds.test.ts | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 app/packages/looker-3d/src/hooks/use-bounds.test.ts diff --git a/app/packages/looker-3d/src/hooks/use-bounds.test.ts b/app/packages/looker-3d/src/hooks/use-bounds.test.ts new file mode 100644 index 0000000000..220cca69c5 --- /dev/null +++ b/app/packages/looker-3d/src/hooks/use-bounds.test.ts @@ -0,0 +1,97 @@ +import { act, renderHook } from "@testing-library/react-hooks"; +import { Box3, Group } from "three"; +import { afterEach, describe, expect, it, Mock, vi } from "vitest"; +import { useFo3dBounds } from "./use-bounds"; + +vi.useFakeTimers(); + +vi.mock("three", () => { + return { + Box3: vi.fn(), + }; +}); + +describe("useFo3dBounds", () => { + afterEach(() => { + vi.clearAllTimers(); + vi.resetAllMocks(); + }); + + it("does not set bounding box when objectRef.current is null", () => { + const objectRef = { current: null } as React.RefObject; + + const { result } = renderHook(() => useFo3dBounds(objectRef)); + + expect(result.current).toBeNull(); + + act(() => { + vi.advanceTimersByTime(500); + }); + + expect(result.current).toBeNull(); + }); + + it("sets bounding box when bounding box stabilizes", () => { + const objectRef = { current: {} } as React.RefObject; + + let callCount = 0; + + // mock Box3 to return changing boxes initially, + // then stable boxes + const boxes = [ + // changing boxes + { + min: { x: 0, y: 0, z: 0, equals: vi.fn(() => false) }, + max: { x: 1, y: 1, z: 1, equals: vi.fn(() => false) }, + }, + // stable boxes + { + min: { x: 0.5, y: 0.5, z: 0.5, equals: vi.fn(() => true) }, + max: { x: 1.5, y: 1.5, z: 1.5, equals: vi.fn(() => true) }, + }, + ]; + + const MockBox3 = vi.fn().mockImplementation(() => { + const box = boxes[callCount < 5 ? 0 : 1]; + callCount++; + return { + min: box.min, + max: box.max, + setFromObject: vi.fn().mockReturnThis(), + }; + }); + + // set the implementation of the mocked Box3 + (Box3 as unknown as Mock).mockImplementation(MockBox3); + + const { result } = renderHook(() => useFo3dBounds(objectRef)); + + expect(result.current).toBeNull(); + + act(() => { + for (let i = 0; i < 10; i++) { + vi.advanceTimersByTime(50); + } + }); + + expect(result.current).not.toBeNull(); + expect(result.current.min).toEqual(boxes[1].min); + expect(result.current.max).toEqual(boxes[1].max); + }); + + it("does not proceed if predicate returns false", () => { + const objectRef = { current: {} } as React.RefObject; + const predicate = vi.fn(() => false); + + const { result } = renderHook(() => useFo3dBounds(objectRef, predicate)); + + expect(result.current).toBeNull(); + expect(predicate).toHaveBeenCalled(); + + act(() => { + vi.advanceTimersByTime(500); + }); + + expect(result.current).toBeNull(); + }); +}); From 916a436b56c4995326a1a0674adf42cfd4e9b6ef Mon Sep 17 00:00:00 2001 From: Sashank Aryal Date: Wed, 16 Oct 2024 21:28:00 +0545 Subject: [PATCH 24/69] add help text --- app/packages/looker-3d/src/action-bar/ViewHelp.tsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/packages/looker-3d/src/action-bar/ViewHelp.tsx b/app/packages/looker-3d/src/action-bar/ViewHelp.tsx index 24b61c2004..9b11927729 100644 --- a/app/packages/looker-3d/src/action-bar/ViewHelp.tsx +++ b/app/packages/looker-3d/src/action-bar/ViewHelp.tsx @@ -20,6 +20,11 @@ export const LOOKER3D_HELP_ITEMS = [ { shortcut: "B", title: "Background", detail: "Toggle background" }, { shortcut: "C", title: "Controls", detail: "Toggle controls" }, { shortcut: "G", title: "Grid", detail: "Toggle grid" }, + { + shortcut: "Z", + title: "Crop", + detail: "Crop and set camera look-at on visible labels", + }, { shortcut: "F", title: "Full-screen", detail: "Toggle full-screen" }, { shortcut: "J", title: "Json ", detail: "Toggle JSON view" }, { shortcut: "I", title: "FO3D ", detail: "Toggle FO3D JSON view" }, From 728542f1d9c46d8c2e823f0f6b11f763b6284f53 Mon Sep 17 00:00:00 2001 From: Sashank Aryal Date: Mon, 14 Oct 2024 23:04:21 +0545 Subject: [PATCH 25/69] upgrade 3d dependencies --- app/packages/looker-3d/package.json | 10 ++-- app/yarn.lock | 91 ++++++++++++++++++----------- 2 files changed, 63 insertions(+), 38 deletions(-) diff --git a/app/packages/looker-3d/package.json b/app/packages/looker-3d/package.json index cb5a46c4b1..7641c9b4df 100644 --- a/app/packages/looker-3d/package.json +++ b/app/packages/looker-3d/package.json @@ -16,14 +16,14 @@ "dependencies": { "@emotion/react": "^11.11.3", "@emotion/styled": "^11.11.0", - "@react-three/drei": "^9.107.2", - "@react-three/fiber": "^8.16.8", + "@react-three/drei": "^9.114.4", + "@react-three/fiber": "^8.17.10", "leva": "^0.9.35", "lodash": "^4.17.21", - "r3f-perf": "^7.2.1", + "r3f-perf": "^7.2.2", "react-color": "^2.19.3", "styled-components": "^6.1.8", - "three": "^0.165.0", + "three": "^0.169.0", "tunnel-rat": "^0.1.2" }, "devDependencies": { @@ -31,7 +31,7 @@ "@types/node": "^20.11.10", "@types/react": "^18.2.48", "@types/react-dom": "^18.2.18", - "@types/three": "^0.165.0", + "@types/three": "^0.169.0", "@vitejs/plugin-react": "^1.3.0", "nodemon": "^3.0.3", "rollup-plugin-external-globals": "^0.6.1", diff --git a/app/yarn.lock b/app/yarn.lock index 86e96c90f8..9771ba0e5e 100644 --- a/app/yarn.lock +++ b/app/yarn.lock @@ -1835,21 +1835,21 @@ __metadata: "@biomejs/biome": ^1.8.3 "@emotion/react": ^11.11.3 "@emotion/styled": ^11.11.0 - "@react-three/drei": ^9.107.2 - "@react-three/fiber": ^8.16.8 + "@react-three/drei": ^9.114.4 + "@react-three/fiber": ^8.17.10 "@types/node": ^20.11.10 "@types/react": ^18.2.48 "@types/react-dom": ^18.2.18 - "@types/three": ^0.165.0 + "@types/three": ^0.169.0 "@vitejs/plugin-react": ^1.3.0 leva: ^0.9.35 lodash: ^4.17.21 nodemon: ^3.0.3 - r3f-perf: ^7.2.1 + r3f-perf: ^7.2.2 react-color: ^2.19.3 rollup-plugin-external-globals: ^0.6.1 styled-components: ^6.1.8 - three: ^0.165.0 + three: ^0.169.0 tunnel-rat: ^0.1.2 typescript: ^5.4.5 vite: ^5.2.14 @@ -3727,9 +3727,9 @@ __metadata: languageName: node linkType: hard -"@react-three/drei@npm:^9.107.2": - version: 9.107.2 - resolution: "@react-three/drei@npm:9.107.2" +"@react-three/drei@npm:^9.114.4": + version: 9.114.4 + resolution: "@react-three/drei@npm:9.114.4" dependencies: "@babel/runtime": ^7.11.2 "@mediapipe/tasks-vision": 0.10.8 @@ -3747,7 +3747,7 @@ __metadata: stats-gl: ^2.0.0 stats.js: ^0.17.0 suspend-react: ^0.1.3 - three-mesh-bvh: ^0.7.0 + three-mesh-bvh: ^0.7.8 three-stdlib: ^2.29.9 troika-three-text: ^0.49.0 tunnel-rat: ^0.1.2 @@ -3762,22 +3762,23 @@ __metadata: peerDependenciesMeta: react-dom: optional: true - checksum: 4a0c4fdaf88c43098719539d7d237e663c16d9b9a6ed95151617691836253b260a865e59345e04cb7311547608472d92ad0cb181c3d4b8d6ea7060a119e6fd3d + checksum: d3ec8e40f5db14129b6d6549aa8bb3056811f17a97859cc770fb89530932ac01fac0e82a51fa7ce267bd5e0dd6f492d8ab67bc0fd899d9352227831c3d73fe86 languageName: node linkType: hard -"@react-three/fiber@npm:^8.16.8": - version: 8.16.8 - resolution: "@react-three/fiber@npm:8.16.8" +"@react-three/fiber@npm:^8.17.10": + version: 8.17.10 + resolution: "@react-three/fiber@npm:8.17.10" dependencies: "@babel/runtime": ^7.17.8 + "@types/debounce": ^1.2.1 "@types/react-reconciler": ^0.26.7 "@types/webxr": "*" base64-js: ^1.5.1 buffer: ^6.0.3 + debounce: ^1.2.1 its-fine: ^1.0.6 react-reconciler: ^0.27.0 - react-use-measure: ^2.1.1 scheduler: ^0.21.0 suspend-react: ^0.1.3 zustand: ^3.7.1 @@ -3803,7 +3804,7 @@ __metadata: optional: true react-native: optional: true - checksum: eb675bdb6af8ef6d7d60d34d755e03ee0b4458f5d45b6f4445e55dc9f80873e3156ce7ce87eb31379e2a7a7625530a9a0742d741daef53a35b106318791c4b51 + checksum: 353c921f95c904883412cacefd8043cd864cb34e748b3195419064bb6577b14ec6d5f5e1f087b52a198a555b3f29e9a9196e32e8b25f8e6f14ec1e80211d81ae languageName: node linkType: hard @@ -4395,10 +4396,10 @@ __metadata: languageName: node linkType: hard -"@tweenjs/tween.js@npm:~23.1.1": - version: 23.1.1 - resolution: "@tweenjs/tween.js@npm:23.1.1" - checksum: a93b331f0c2dc2dd2fce2c08ab9e505cd71ce1b31a3e2218f31da4f2d2af84ac4c4eaab05f9929a7b7884c5e00c5d32e8fd1872e0a3e10e9cca20febc8d263b5 +"@tweenjs/tween.js@npm:~23.1.3": + version: 23.1.3 + resolution: "@tweenjs/tween.js@npm:23.1.3" + checksum: 2f8a908b275bb6729bde4b863c277bf7411d2e0302ceb0455369479077b89eaf8380cd9206b91ff574416418a95c6f06db4e1ddea732a286d0db0ba8e7c093d3 languageName: node linkType: hard @@ -4526,6 +4527,13 @@ __metadata: languageName: node linkType: hard +"@types/debounce@npm:^1.2.1": + version: 1.2.4 + resolution: "@types/debounce@npm:1.2.4" + checksum: decef3eee65d681556d50f7fac346f1b33134f6b21f806d41326f9dfb362fa66b0282ff0640ae6791b690694c9dc3dad4e146e909e707e6f96650f3aa325b9da + languageName: node + linkType: hard + "@types/debug@npm:^4.0.0": version: 4.1.12 resolution: "@types/debug@npm:4.1.12" @@ -4988,16 +4996,17 @@ __metadata: languageName: node linkType: hard -"@types/three@npm:^0.165.0": - version: 0.165.0 - resolution: "@types/three@npm:0.165.0" +"@types/three@npm:^0.169.0": + version: 0.169.0 + resolution: "@types/three@npm:0.169.0" dependencies: - "@tweenjs/tween.js": ~23.1.1 + "@tweenjs/tween.js": ~23.1.3 "@types/stats.js": "*" "@types/webxr": "*" + "@webgpu/types": "*" fflate: ~0.8.2 meshoptimizer: ~0.18.1 - checksum: ed86979c5cee78070b9fe9fd94f6049d27c392eeb3061f10fc4b54b85daae2433b6f184c1a00407cbf856778c5bc113fdf46b8b87e778ed91a311e06b9ae938b + checksum: faaa5bbcead150b5627e2e00bb0e63d98c2a8f8d8951f729f889f430a98534c568d73520f43263391157f216a765806c16e2ca05b688506bffb7f8cf417be042 languageName: node linkType: hard @@ -5499,6 +5508,13 @@ __metadata: languageName: node linkType: hard +"@webgpu/types@npm:*": + version: 0.1.48 + resolution: "@webgpu/types@npm:0.1.48" + checksum: d9976049bc92615396e23f78a59554a7aff9dbc99c49696903cd4a2a18dd2bb5fd6d3a67196fc195ae9ae8b3d7f25c5060ea5d77c5dd6eac34dfbc0adbdf9648 + languageName: node + linkType: hard + "@xmldom/xmldom@npm:^0.8.6": version: 0.8.10 resolution: "@xmldom/xmldom@npm:0.8.10" @@ -14128,9 +14144,9 @@ __metadata: languageName: node linkType: hard -"r3f-perf@npm:^7.2.1": - version: 7.2.1 - resolution: "r3f-perf@npm:7.2.1" +"r3f-perf@npm:^7.2.2": + version: 7.2.2 + resolution: "r3f-perf@npm:7.2.2" dependencies: "@radix-ui/react-icons": ^1.3.0 "@react-three/drei": ^9.103.0 @@ -14149,7 +14165,7 @@ __metadata: optional: true react-dom: optional: true - checksum: a4fa7f636861ad8de46e48cd245aed7634ee3a5b76e63c573a98bbfb0cf6d57aadb81821c43acd2df453bec9c4fd6e520a5c650a25b21d2d79ef1a62034c5ec3 + checksum: ee1b7a6e698d670b1467c6e1dbffaff36de008cfcdd5a5fff87f0b0da7c99f0db04a111320aa2850851881bbe954401a506091ef0c3f32c897f4faaebc7fbae3 languageName: node linkType: hard @@ -14549,7 +14565,7 @@ __metadata: languageName: node linkType: hard -"react-use-measure@npm:^2.0.1, react-use-measure@npm:^2.1.1": +"react-use-measure@npm:^2.0.1": version: 2.1.1 resolution: "react-use-measure@npm:2.1.1" dependencies: @@ -16362,6 +16378,15 @@ __metadata: languageName: node linkType: hard +"three-mesh-bvh@npm:^0.7.8": + version: 0.7.8 + resolution: "three-mesh-bvh@npm:0.7.8" + peerDependencies: + three: ">= 0.151.0" + checksum: 3023a7d1ccdf033e08eba5c6437b4c3dd31a6ec83cb54cfadfca2d6aa22c74c7895c5b0d6e943897ac411a64b86d9bf78b3f6c3a902efd8b8bfd3cf0afd9a18e + languageName: node + linkType: hard + "three-stdlib@npm:^2.29.4": version: 2.29.5 resolution: "three-stdlib@npm:2.29.5" @@ -16394,10 +16419,10 @@ __metadata: languageName: node linkType: hard -"three@npm:^0.165.0": - version: 0.165.0 - resolution: "three@npm:0.165.0" - checksum: 1dcfe9ef046a49036e527cf0f72e07475546a16ede694f24630330ece7635ab8ee5c3a3422d43c65c2d267a255e4859820ddb35381defadbd3515d91e2c2bd2e +"three@npm:^0.169.0": + version: 0.169.0 + resolution: "three@npm:0.169.0" + checksum: 714b68911a26523858edf10e6ed33e41901edaba734b6acd3aa3542746e45570295fa8750723b06bcb9d223013dba920e946e02a018ee136c86da927c04441c3 languageName: node linkType: hard From 2af174a19539c2deb182fa09698f1ed3384b1347 Mon Sep 17 00:00:00 2001 From: Sashank Aryal Date: Wed, 16 Oct 2024 21:42:32 +0545 Subject: [PATCH 26/69] account for detections --- .../looker-3d/src/fo3d/MediaTypeFo3d.tsx | 42 +++++++++++++++++-- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/app/packages/looker-3d/src/fo3d/MediaTypeFo3d.tsx b/app/packages/looker-3d/src/fo3d/MediaTypeFo3d.tsx index 331928bea4..c1da5f5a47 100644 --- a/app/packages/looker-3d/src/fo3d/MediaTypeFo3d.tsx +++ b/app/packages/looker-3d/src/fo3d/MediaTypeFo3d.tsx @@ -383,16 +383,50 @@ export const MediaTypeFo3dComponent = () => { const currentSelectedLabels = await snapshot.getPromise( fos.selectedLabels ); - const labelsFieldNames = currentSelectedLabels.map((l) => l.field); + + if (currentSelectedLabels.length === 0) { + return; + } const labelBoundingBoxes = []; - for (const labelKey of labelsFieldNames) { - const thisLabelDimension = sample.sample[labelKey].dimensions as [ + + for (const selectedLabel of currentSelectedLabels) { + const field = selectedLabel.field; + const labelId = selectedLabel.labelId; + + const labelFieldData = sample.sample[field]; + + let thisLabel = null; + + if (Array.isArray(labelFieldData)) { + // if the field data is an array of labels + thisLabel = labelFieldData.find( + (l) => l._id === labelId || l.id === labelId + ); + } else if ( + labelFieldData && + labelFieldData.detections && + Array.isArray(labelFieldData.detections) + ) { + // if the field data contains detections + thisLabel = labelFieldData.detections.find( + (l) => l._id === labelId || l.id === labelId + ); + } else { + // single label + thisLabel = labelFieldData; + } + + if (!thisLabel) { + continue; + } + + const thisLabelDimension = thisLabel.dimensions as [ number, number, number ]; - const thisLabelLocation = sample.sample[labelKey].location as [ + const thisLabelLocation = thisLabel.location as [ number, number, number From e5c2f7cefec964dfde8409374103ff2c64b3b42d Mon Sep 17 00:00:00 2001 From: Benjamin Kane Date: Wed, 16 Oct 2024 13:16:58 -0400 Subject: [PATCH 27/69] min font size is 14 (#4933) --- app/packages/core/src/components/Grid/useFontSize.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/packages/core/src/components/Grid/useFontSize.ts b/app/packages/core/src/components/Grid/useFontSize.ts index 25337c9dc0..f443a87391 100644 --- a/app/packages/core/src/components/Grid/useFontSize.ts +++ b/app/packages/core/src/components/Grid/useFontSize.ts @@ -2,7 +2,7 @@ import { useCallback } from "react"; import useThreshold from "./useThreshold"; const MAX = 32; -const MIN = 10; +const MIN = 14; const SCALE_FACTOR = 0.09; export default (id: string) => { From 05eb55bb4e461c7213e1c2690b9d1ede03e8bff4 Mon Sep 17 00:00:00 2001 From: Sashank Aryal Date: Wed, 16 Oct 2024 23:05:11 +0545 Subject: [PATCH 28/69] clear timeout in usebounds --- .../looker-3d/src/hooks/use-bounds.ts | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/app/packages/looker-3d/src/hooks/use-bounds.ts b/app/packages/looker-3d/src/hooks/use-bounds.ts index fc28a93095..c38fbb9b8a 100644 --- a/app/packages/looker-3d/src/hooks/use-bounds.ts +++ b/app/packages/looker-3d/src/hooks/use-bounds.ts @@ -21,6 +21,8 @@ export const useFo3dBounds = ( const unchangedCount = useRef(0); const previousBox = useRef(null); + const timeOutIdRef = useRef(null); + useLayoutEffect(() => { if (predicate && !predicate()) { return; @@ -37,14 +39,20 @@ export const useFo3dBounds = ( if (!isMounted) return; if (!objectRef.current) { - setTimeout(getBoundingBox, BOUNDING_BOX_POLLING_INTERVAL); + timeOutIdRef.current = window.setTimeout( + getBoundingBox, + BOUNDING_BOX_POLLING_INTERVAL + ); return; } const box = new Box3().setFromObject(objectRef.current); if (Math.abs(box.max?.x) === Number.POSITIVE_INFINITY) { - setTimeout(getBoundingBox, BOUNDING_BOX_POLLING_INTERVAL); + timeOutIdRef.current = window.setTimeout( + getBoundingBox, + BOUNDING_BOX_POLLING_INTERVAL + ); return; } @@ -59,17 +67,27 @@ export const useFo3dBounds = ( if (unchangedCount.current >= UNCHANGED_COUNT_THRESHOLD) { setSceneBoundingBox(box); } else { - setTimeout(getBoundingBox, BOUNDING_BOX_POLLING_INTERVAL); + timeOutIdRef.current = window.setTimeout( + getBoundingBox, + BOUNDING_BOX_POLLING_INTERVAL + ); } }; // this is a hack, yet to find a better way than polling to know when the scene is done loading // callbacks in loaders are not reliable - setTimeout(getBoundingBox, BOUNDING_BOX_POLLING_INTERVAL); + timeOutIdRef.current = window.setTimeout( + getBoundingBox, + BOUNDING_BOX_POLLING_INTERVAL + ); // cleanup function to prevent memory leaks return () => { isMounted = false; + + if (timeOutIdRef.current) { + window.clearTimeout(timeOutIdRef.current); + } }; }, [objectRef, predicate]); From 6d84b449a48a99d6bb3c19b75d9c75d34ed3f004 Mon Sep 17 00:00:00 2001 From: brimoor Date: Thu, 17 Oct 2024 09:22:51 -0400 Subject: [PATCH 29/69] documenting ctx.user --- docs/source/plugins/developing_plugins.rst | 3 +++ fiftyone/operators/executor.py | 13 ++++++++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/docs/source/plugins/developing_plugins.rst b/docs/source/plugins/developing_plugins.rst index 68b9a1c12a..52a0f534ba 100644 --- a/docs/source/plugins/developing_plugins.rst +++ b/docs/source/plugins/developing_plugins.rst @@ -981,6 +981,9 @@ contains the following properties: - `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.user` - an object of information about the user that invoked the + operator, if known, including the user's `id`, `name`, `email`, `role`, and + `dataset_permission` - `ctx.panel_id` - the ID of the panel that invoked the operator, if any - `ctx.panel` - a :class:`PanelRef ` instance that you can use to read and write the :ref:`state ` diff --git a/fiftyone/operators/executor.py b/fiftyone/operators/executor.py index e0e6ac784c..a2e8e728c9 100644 --- a/fiftyone/operators/executor.py +++ b/fiftyone/operators/executor.py @@ -385,7 +385,7 @@ async def resolve_type(registry, operator_uri, request_params): return ExecutionResult(error=traceback.format_exc()) -async def resolve_type_with_context(request_params, target: str = None): +async def resolve_type_with_context(request_params, target=None): """Resolves the "inputs" or "outputs" schema of an operator with the given context. @@ -486,7 +486,7 @@ def __init__( self._dataset = None self._view = None self._ops = Operations(self) - self.user = None + self._user = None self._set_progress = set_progress self._delegated_operation_id = delegated_operation_id @@ -646,7 +646,14 @@ def current_sample(self): @property def user_id(self): """The ID of the user executing the operation, if known.""" - return self.user.id if self.user else None + return self._user.id if self._user else None + + @property + def user(self): + """An object of information about the user executing the operation, if + known. + """ + return self._user @property def panel_id(self): From 1e3bb861d122ab1d191724dcbfb31d10bf346aad Mon Sep 17 00:00:00 2001 From: brimoor Date: Thu, 17 Oct 2024 09:36:18 -0400 Subject: [PATCH 30/69] adding user_request_token --- docs/source/plugins/developing_plugins.rst | 11 +++++++---- fiftyone/operators/executor.py | 7 +++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/docs/source/plugins/developing_plugins.rst b/docs/source/plugins/developing_plugins.rst index 52a0f534ba..ceab2dcc5f 100644 --- a/docs/source/plugins/developing_plugins.rst +++ b/docs/source/plugins/developing_plugins.rst @@ -980,10 +980,13 @@ contains the following properties: 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.user` - an object of information about the user that invoked the - operator, if known, including the user's `id`, `name`, `email`, `role`, and - `dataset_permission` +- `ctx.user_id` **(Teams only)** - the ID of the user that invoked the + operator, if known +- `ctx.user` **(Teams only)** - an object of information about the user that + invoked the operator, if known, including the user's `id`, `name`, `email`, + `role`, and `dataset_permission` +- `ctx.user_request_token` **(Teams only)** - the request token + authenticating the user executing the operation, if known - `ctx.panel_id` - the ID of the panel that invoked the operator, if any - `ctx.panel` - a :class:`PanelRef ` instance that you can use to read and write the :ref:`state ` diff --git a/fiftyone/operators/executor.py b/fiftyone/operators/executor.py index a2e8e728c9..a35a1b8d6a 100644 --- a/fiftyone/operators/executor.py +++ b/fiftyone/operators/executor.py @@ -655,6 +655,13 @@ def user(self): """ return self._user + @property + def user_request_token(self): + """The request token authenticating the user executing the operation, + if known. + """ + return self._user._request_token if self._user else None + @property def panel_id(self): """The ID of the panel that invoked the operator, if any.""" From 34bb11c85b6531120c67415883c4076269abf89c Mon Sep 17 00:00:00 2001 From: brimoor Date: Thu, 17 Oct 2024 09:42:20 -0400 Subject: [PATCH 31/69] removing Teams-only tag --- docs/source/plugins/developing_plugins.rst | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/docs/source/plugins/developing_plugins.rst b/docs/source/plugins/developing_plugins.rst index ceab2dcc5f..7083a06c4b 100644 --- a/docs/source/plugins/developing_plugins.rst +++ b/docs/source/plugins/developing_plugins.rst @@ -980,13 +980,12 @@ contains the following properties: 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` **(Teams only)** - the ID of the user that invoked the - operator, if known -- `ctx.user` **(Teams only)** - an object of information about the user that - invoked the operator, if known, including the user's `id`, `name`, `email`, - `role`, and `dataset_permission` -- `ctx.user_request_token` **(Teams only)** - the request token - authenticating the user executing the operation, if known +- `ctx.user_id` - the ID of the user that invoked the operator, if known +- `ctx.user` - an object of information about the user that invoked the + operator, if known, including the user's `id`, `name`, `email`, `role`, and + `dataset_permission` +- `ctx.user_request_token` - the request token authenticating the user + executing the operation, if known - `ctx.panel_id` - the ID of the panel that invoked the operator, if any - `ctx.panel` - a :class:`PanelRef ` instance that you can use to read and write the :ref:`state ` From 85df5a0d0a6d2ef40302a90703f46ff7dc654f89 Mon Sep 17 00:00:00 2001 From: Jacob Marks Date: Thu, 17 Oct 2024 10:22:21 -0400 Subject: [PATCH 32/69] set model to eval model --- fiftyone/utils/open_clip.py | 1 + 1 file changed, 1 insertion(+) diff --git a/fiftyone/utils/open_clip.py b/fiftyone/utils/open_clip.py index 4824248443..f462284289 100644 --- a/fiftyone/utils/open_clip.py +++ b/fiftyone/utils/open_clip.py @@ -95,6 +95,7 @@ def _load_model(self, config): device=self.device, ) self._tokenizer = open_clip.get_tokenizer(config.clip_model) + self._model.eval() return self._model def _get_text_features(self): From 45806307d733dc900d68177b48f4a006bb1e83b6 Mon Sep 17 00:00:00 2001 From: Jacob Marks Date: Thu, 17 Oct 2024 10:22:49 -0400 Subject: [PATCH 33/69] document open_clip eval() mode --- docs/source/integrations/openclip.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/source/integrations/openclip.rst b/docs/source/integrations/openclip.rst index a3f2653564..47ac3e66fb 100644 --- a/docs/source/integrations/openclip.rst +++ b/docs/source/integrations/openclip.rst @@ -88,6 +88,11 @@ When running inference with OpenCLIP, you can specify a text prompt to help guide the model towards a solution as well as only specify a certain number of classes to output during zero shot classification. +.. note:: + + While OpenCLIP models are typically set to train mode by default, the FiftyOne + integration sets the model to eval mode before running inference. + For example we can run inference as such: .. code-block:: python From 59702204148ecf857a13883ddf0f648d4c98aa92 Mon Sep 17 00:00:00 2001 From: Jacob Marks Date: Thu, 17 Oct 2024 10:28:28 -0400 Subject: [PATCH 34/69] update amp autocast --- fiftyone/utils/open_clip.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fiftyone/utils/open_clip.py b/fiftyone/utils/open_clip.py index f462284289..6c0f4cd253 100644 --- a/fiftyone/utils/open_clip.py +++ b/fiftyone/utils/open_clip.py @@ -145,7 +145,7 @@ def _predict_all(self, imgs): if self._using_gpu: imgs = imgs.cuda() - with torch.no_grad(), torch.cuda.amp.autocast(): + with torch.no_grad(), torch.amp.autocast("cuda"): image_features = self._model.encode_image(imgs) text_features = self._get_text_features() From 67fe3d601703b8f7f74eed09c5d3426fc438bccf Mon Sep 17 00:00:00 2001 From: Jacob Marks Date: Thu, 17 Oct 2024 10:39:31 -0400 Subject: [PATCH 35/69] update HF dataset repo ids --- docs/source/integrations/huggingface.rst | 58 ++++++++++++------------ 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/docs/source/integrations/huggingface.rst b/docs/source/integrations/huggingface.rst index ac553e62bc..e7fd8851d0 100644 --- a/docs/source/integrations/huggingface.rst +++ b/docs/source/integrations/huggingface.rst @@ -1309,7 +1309,7 @@ If the repo was uploaded to the Hugging Face Hub via FiftyOne's :func:`push_to_hub() ` function, then the `fiftyone.yml` config file will be generated and uploaded to the repo. However, some common datasets like -`mnist `_ were uploaded to the Hub +`mnist `_ were uploaded to the Hub using the `datasets` library and do not contain a `fiftyone.yml` or `fiftyone.yaml` file. If you know how the dataset is structured, you can load the dataset by passing the path to a local yaml config file that describes the @@ -1332,7 +1332,7 @@ the path to the local yaml config file: from fiftyone.utils.huggingface import load_from_hub dataset = load_from_hub( - "mnist", + "ylecun/mnist", config_file="/path/to/mnist.yml", ) @@ -1360,7 +1360,7 @@ and `classification_fields` arguments directly: from fiftyone.utils.huggingface import load_from_hub dataset = load_from_hub( - "mnist", + "ylecun/mnist", format="ParquetFilesDataset", classification_fields="label", ) @@ -1400,7 +1400,7 @@ Let's look at these categories in more detail: dataset that are *compatible* with this config, and are *available* to be loaded. In Hugging Face, the "dataset" in a repo can contain multiple "subsets", which may or may not have the same schema. Take the - `Street View House Numbers `_ dataset for + `Street View House Numbers `_ dataset for example. This dataset has two subsets: `"cropped_digits"` and `"full_numbers"`. The `cropped_digits` subset contains classification labels, while the `full_numbers` subset contains detection labels. A single config would not be @@ -1419,7 +1419,7 @@ Let's look at these categories in more detail: identifies the names of all splits and by default, will assume that all of these splits are to be loaded. If you only want to load a specific split or splits, you can specify them with the `splits` field. For example, to load the - training split of the `CIFAR10 `_ + training split of the `CIFAR10 `_ dataset, you can pass `splits="train"`. If you want to load multiple splits, you can pass them as a list, e.g., `splits=["train", "test"]`. Note that this is not a required field, and by default all splits are loaded. @@ -1554,8 +1554,8 @@ easy it is in practice to load datasets from the Hugging Face Hub. **Classification Datasets** Let's start by loading the -`MNIST `_ dataset into FiftyOne. All you -need to do is pass the `repo_id` of the dataset — in this case `"mnist"` — to +`MNIST `_ dataset into FiftyOne. All you +need to do is pass the `repo_id` of the dataset — in this case `"ylecun/mnist"` — to :func:`load_from_hub() `, specify the format as `"parquet"`, and specify the `classification_fields` as `"label"`: @@ -1565,7 +1565,7 @@ format as `"parquet"`, and specify the `classification_fields` as `"label"`: from fiftyone.utils.huggingface import load_from_hub dataset = load_from_hub( - "mnist", + "ylecun/mnist", format="parquet", classification_fields="label", max_samples=1000, @@ -1574,25 +1574,25 @@ format as `"parquet"`, and specify the `classification_fields` as `"label"`: session = fo.launch_app(dataset) The same exact syntax works for the `CIFAR-10 `_ -and `FashionMNIST `_ datasets, +and `FashionMNIST `_ datasets, which are also available on the Hub. In fact, you can load any of the following classification datasets from the Hub using the same syntax, just by changing the `repo_id`: -- `CIFAR-10 `_ (use `"cifar10"`) -- `ImageNet `_ (use `"imagenet-1k"`) -- `FashionMNIST `_ (use `"fashion_mnist"`) +- `CIFAR-10 `_ (use `"uoft-cs/cifar10"`) +- `ImageNet `_ (use `"ILSVRC/imagenet-1k"`) +- `FashionMNIST `_ (use `"zalando-datasets/fashion_mnist"`) - `Tiny ImageNet `_ (use `"zh-plus/tiny-imagenet"`) -- `Food-101 `_ (use `"food101"`) -- `Dog Food `_ (use `"sasha/dog-food"`) -- `ImageNet-Sketch `_ (use `"imagenet_sketch"`) +- `Food-101 `_ (use `"ethz/food101"`) +- `Dog Food `_ (use `"sasha/dog-food"`) +- `ImageNet-Sketch `_ (use `"songweig/imagenet_sketch"`) - `Oxford Flowers `_ (use `"nelorth/oxford-flowers"`) -- `Cats vs. Dogs `_ (use `"cats_vs_dogs"`) +- `Cats vs. Dogs `_ (use `"microsoft/cats_vs_dogs"`) - `ObjectNet-1.0 `_ (use `"timm/objectnet"`) A very similar syntax can be used to load classification datasets that contain *multiple* classification fields, such as -`CIFAR-100 `_ and the +`CIFAR-100 `_ and the `WikiArt `_ dataset. For example, to load the CIFAR-100 dataset, you can specify the `classification_fields` as `["coarse_label", "fine_label"]`: @@ -1603,7 +1603,7 @@ to load the CIFAR-100 dataset, you can specify the `classification_fields` as from fiftyone.utils.huggingface import load_from_hub dataset = load_from_hub( - "cifar100", + "uoft-cs/cifar100", format="parquet", classification_fields=["coarse_label", "fine_label"], max_samples=1000, @@ -1638,7 +1638,7 @@ dataset. For example, to load the `cropped_digits` subset of the from fiftyone.utils.huggingface import load_from_hub dataset = load_from_hub( - "svhn", + "ufldl-stanford/svhn", format="parquet", classification_fields="label", subsets="cropped_digits", @@ -1671,8 +1671,8 @@ standard column name for detection features in Hugging Face datasets: The same syntax works for many other popular detection datasets on the Hub, including: -- `CPPE - 5 `_ (use `"cppe-5"`) -- `WIDER FACE `_ (use `"wider_face"`) +- `CPPE - 5 `_ (use `"rishitdagli/cppe-5"`) +- `WIDER FACE `_ (use `"CUHK-CSE/wider_face"`) - `License Plate Object Detection `_ (use `"keremberke/license-plate-object-detection"`) - `Aerial Sheep Object Detection `_ @@ -1680,7 +1680,7 @@ including: Some detection datasets have their detections stored under a column with a different name. For example, the `full_numbers` subset of the -`Street View House Numbers `_ dataset +`Street View House Numbers `_ dataset stores its detections under the column `digits`. To load this subset, you can specify the `detection_fields` as `"digits"`: @@ -1690,7 +1690,7 @@ specify the `detection_fields` as `"digits"`: from fiftyone.utils.huggingface import load_from_hub dataset = load_from_hub( - "svhn", + "ufldl-stanford/svhn", format="parquet", detection_fields="digits", subsets="full_numbers", @@ -1711,7 +1711,7 @@ specify the `detection_fields` as `"digits"`: Loading segmentation datasets from the Hub is also a breeze. For example, to load the "instance_segmentation" subset from -`SceneParse150 `_, all you +`SceneParse150 `_, all you need to do is specify the `mask_fields` as `"annotation"`: .. code-block:: python @@ -1720,7 +1720,7 @@ need to do is specify the `mask_fields` as `"annotation"`: from fiftyone.utils.huggingface import load_from_hub dataset = load_from_hub( - "scene_parse150", + "zhoubolei/scene_parse150", format="parquet", subsets="instance_segmentation", mask_fields="annotation", @@ -1838,7 +1838,7 @@ need to specify the `filepath` as `"url"`: session = fo.launch_app(dataset) -For `RedCaps `_, we instead use +For `RedCaps `_, we instead use `"image_url"` as the `filepath`: .. code-block:: python @@ -1847,7 +1847,7 @@ For `RedCaps `_, we instead use from fiftyone.utils.huggingface import load_from_hub dataset = load_from_hub( - "red_caps", + "kdexd/red_caps", format="parquet", filepath="image_url", max_samples=1000, @@ -1944,7 +1944,7 @@ Now, you can load the dataset using the local yaml config file: When loading datasets from the Hub, you can customize the download process by specifying the `batch_size`, `num_workers`, and `overwrite` arguments. For example, to download the `full_numbers` subset of the `Street View House Numbers -`_ dataset with a batch size of 50 and 4 +`_ dataset with a batch size of 50 and 4 workers, you can do the following: .. code-block:: python @@ -1953,7 +1953,7 @@ workers, you can do the following: from fiftyone.utils.huggingface import load_from_hub dataset = load_from_hub( - "svhn", + "ufldl-stanford/svhn", format="parquet", detection_fields="digits", subsets="full_numbers", From 8090f931233fb81b929f00fee8fe307d5c348c77 Mon Sep 17 00:00:00 2001 From: brimoor Date: Thu, 17 Oct 2024 10:39:33 -0400 Subject: [PATCH 36/69] must use instance variable --- fiftyone/operators/executor.py | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/fiftyone/operators/executor.py b/fiftyone/operators/executor.py index a35a1b8d6a..3af9f377a9 100644 --- a/fiftyone/operators/executor.py +++ b/fiftyone/operators/executor.py @@ -482,11 +482,11 @@ def __init__( self.request_params = request_params or {} self.params = self.request_params.get("params", {}) self.executor = executor + self.user = None self._dataset = None self._view = None self._ops = Operations(self) - self._user = None self._set_progress = set_progress self._delegated_operation_id = delegated_operation_id @@ -646,21 +646,14 @@ def current_sample(self): @property def user_id(self): """The ID of the user executing the operation, if known.""" - return self._user.id if self._user else None - - @property - def user(self): - """An object of information about the user executing the operation, if - known. - """ - return self._user + return self.user.id if self.user else None @property def user_request_token(self): """The request token authenticating the user executing the operation, if known. """ - return self._user._request_token if self._user else None + return self.user._request_token if self.user else None @property def panel_id(self): From 8feb0c927c0ca4c63d5c0997f769d2a67430064b Mon Sep 17 00:00:00 2001 From: Camron Staley <55006027+CamronStaley@users.noreply.github.com> Date: Thu, 17 Oct 2024 09:45:24 -0500 Subject: [PATCH 37/69] adding results output for execute_queued_ops and execute_ops (#4936) * adding results output for execute_queued_ops and execute_ops * move and refactor delegated_operations_tests * update tests * improve results assertion in do tests; black file --------- Co-authored-by: Stuart Wheaton --- fiftyone/operators/delegated.py | 8 +- .../delegated_tests.py} | 94 ++++++++++++++++--- 2 files changed, 89 insertions(+), 13 deletions(-) rename tests/unittests/{delegated_operators_tests.py => operators/delegated_tests.py} (92%) diff --git a/fiftyone/operators/delegated.py b/fiftyone/operators/delegated.py index 4fd9c53eec..d4ef748473 100644 --- a/fiftyone/operators/delegated.py +++ b/fiftyone/operators/delegated.py @@ -404,6 +404,7 @@ def execute_queued_operations( log (False): the optional boolean flag to log the execution of the delegated operations """ + results = [] if limit is not None: paging = DelegatedOperationPagingParams(limit=limit) else: @@ -419,7 +420,8 @@ def execute_queued_operations( ) for op in queued_ops: - self.execute_operation(operation=op, log=log) + results.append(self.execute_operation(operation=op, log=log)) + return results def count(self, filters=None, search=None): """Counts the delegated operations matching the given criteria. @@ -444,6 +446,7 @@ def execute_operation(self, operation, log=False, run_link=None): run_link (None): an optional link to orchestrator-specific information about the operation """ + result = None try: succeeded = ( self.set_running( @@ -461,7 +464,7 @@ def execute_operation(self, operation, log=False, run_link=None): operation.id, operation.operator, ) - return + return result if log: logger.info( @@ -483,6 +486,7 @@ def execute_operation(self, operation, log=False, run_link=None): logger.info( "Operation %s failed\n%s", operation.id, result.error ) + return result async def _execute_operator(self, doc): operator_uri = doc.operator diff --git a/tests/unittests/delegated_operators_tests.py b/tests/unittests/operators/delegated_tests.py similarity index 92% rename from tests/unittests/delegated_operators_tests.py rename to tests/unittests/operators/delegated_tests.py index a0524bbadc..ef48c78ee9 100644 --- a/tests/unittests/delegated_operators_tests.py +++ b/tests/unittests/operators/delegated_tests.py @@ -55,6 +55,8 @@ def to_json(self): class MockOperator(Operator): + result = {"executed": True} + def __init__(self, success=True, sets_progress=False, **kwargs): self.success = success self.sets_progress = sets_progress @@ -80,7 +82,7 @@ def execute(self, ctx): if self.sets_progress: ctx.set_progress(0.5, "halfway there") - return {"executed": True} + return self.result class MockGeneratorOperator(Operator): @@ -467,7 +469,12 @@ def test_sets_progress( self.docs_to_delete.append(doc) self.assertEqual(doc.run_state, ExecutionRunState.QUEUED) - self.svc.execute_queued_operations(delegation_target="test_target") + results = self.svc.execute_queued_operations( + delegation_target="test_target" + ) + self.assertEqual(len(results), 1) + self.assertIsNone(results[0].error) + self.assertDictEqual(results[0].result, MockOperator.result) doc = self.svc.get(doc_id=doc.id) self.assertEqual(doc.run_state, ExecutionRunState.COMPLETED) @@ -495,7 +502,12 @@ def test_full_run_success( self.docs_to_delete.append(doc) self.assertEqual(doc.run_state, ExecutionRunState.QUEUED) - self.svc.execute_queued_operations(delegation_target="test_target") + results = self.svc.execute_queued_operations( + delegation_target="test_target" + ) + self.assertEqual(len(results), 1) + self.assertIsNone(results[0].error) + self.assertDictEqual(results[0].result, MockOperator.result) doc = self.svc.get(doc_id=doc.id) self.assertEqual(doc.run_state, ExecutionRunState.COMPLETED) @@ -529,9 +541,11 @@ def test_generator_run_success( self.docs_to_delete.append(doc) self.assertEqual(doc.run_state, ExecutionRunState.QUEUED) - self.svc.execute_queued_operations( + results = self.svc.execute_queued_operations( delegation_target="test_target_generator" ) + self.assertEqual(len(results), 1) + self.assertIsNone(results[0].error) doc = self.svc.get(doc_id=doc.id) self.assertEqual(doc.run_state, ExecutionRunState.COMPLETED) @@ -561,7 +575,11 @@ def test_generator_sets_progress( self.docs_to_delete.append(doc) self.assertEqual(doc.run_state, ExecutionRunState.QUEUED) - self.svc.execute_queued_operations(delegation_target="test_target") + results = self.svc.execute_queued_operations( + delegation_target="test_target" + ) + self.assertEqual(len(results), 1) + self.assertIsNone(results[0].error) doc = self.svc.get(doc_id=doc.id) self.assertEqual(doc.run_state, ExecutionRunState.COMPLETED) @@ -664,7 +682,12 @@ def test_full_run_fail( self.docs_to_delete.append(doc) self.assertEqual(doc.run_state, ExecutionRunState.QUEUED) - self.svc.execute_queued_operations(delegation_target="test_target") + results = self.svc.execute_queued_operations( + delegation_target="test_target" + ) + self.assertEqual(len(results), 1) + self.assertIsNotNone(results[0].error) + self.assertIsNone(results[0].result) doc = self.svc.get(doc_id=doc.id) self.assertEqual(doc.run_state, ExecutionRunState.FAILED) @@ -701,7 +724,12 @@ def test_rerun_failed( self.docs_to_delete.append(doc) self.assertEqual(doc.run_state, ExecutionRunState.QUEUED) - self.svc.execute_queued_operations(delegation_target="test_target") + results = self.svc.execute_queued_operations( + delegation_target="test_target" + ) + self.assertEqual(len(results), 1) + self.assertIsNotNone(results[0].error) + self.assertIsNone(results[0].result) doc = self.svc.get(doc_id=doc.id) self.assertEqual(doc.run_state, ExecutionRunState.FAILED) @@ -720,7 +748,12 @@ def test_rerun_failed( self.assertIsNone(rerun_doc.completed_at) self.assertIsNone(rerun_doc.result) - self.svc.execute_queued_operations(delegation_target="test_target") + results = self.svc.execute_queued_operations( + delegation_target="test_target" + ) + self.assertEqual(len(results), 1) + self.assertIsNone(results[0].error) + self.assertDictEqual(results[0].result, MockOperator.result) doc = self.svc.get(doc_id=rerun_doc.id) self.assertEqual(doc.run_state, ExecutionRunState.COMPLETED) @@ -752,7 +785,13 @@ def test_rerun_with_renamed_dataset(self, get_op_mock, op_exists_mock): self.assertEqual(doc.run_state, ExecutionRunState.QUEUED) # Execute once with original dataset name - self.svc.execute_queued_operations(delegation_target="test_target") + results = self.svc.execute_queued_operations( + delegation_target="test_target" + ) + self.assertEqual(len(results), 1) + self.assertIsNotNone(results[0].error) + self.assertIsNone(results[0].result) + doc = self.svc.get(doc_id=doc.id) self.assertEqual(doc.run_state, ExecutionRunState.FAILED) @@ -778,7 +817,12 @@ def test_rerun_with_renamed_dataset(self, get_op_mock, op_exists_mock): self.assertIsNone(rerun_doc.completed_at) self.assertIsNone(rerun_doc.result) - self.svc.execute_queued_operations(delegation_target="test_target") + results = self.svc.execute_queued_operations( + delegation_target="test_target" + ) + self.assertEqual(len(results), 1) + self.assertIsNone(results[0].error) + self.assertDictEqual(results[0].result, MockOperator.result) doc = self.svc.get(doc_id=rerun_doc.id) self.assertEqual(doc.run_state, ExecutionRunState.COMPLETED) @@ -790,6 +834,28 @@ def test_rerun_with_renamed_dataset(self, get_op_mock, op_exists_mock): finally: dataset.delete() + @patch( + "fiftyone.core.odm.utils.load_dataset", + ) + def test_execute_with_already_processing_op( + self, mock_load_dataset, mock_get_operator, mock_operator_exists + ): + mock_load_dataset.return_value = MockDataset() + doc = self.svc.queue_operation( + operator="@voxelfiftyone/operator/foo", + label=mock_get_operator.return_value.name, + delegation_target=f"test_target", + context=ExecutionContext( + request_params={"foo": "bar", "dataset_id": str(ObjectId())} + ), + ) + self.docs_to_delete.append(doc) + doc = self.svc.set_running(doc.id) + result = self.svc.execute_operation(doc) + changed_doc = self.svc.get(doc_id=doc.id) + self.assertEqual(changed_doc.status, doc.status) + self.assertIsNone(result) + def test_execute_with_renamed_dataset(self, get_op_mock, op_exists_mock): # setup uid = str(ObjectId()) @@ -822,7 +888,13 @@ def test_execute_with_renamed_dataset(self, get_op_mock, op_exists_mock): # Execute queued operation after saving the new dataset name try: - self.svc.execute_queued_operations(delegation_target="test_target") + results = self.svc.execute_queued_operations( + delegation_target="test_target" + ) + self.assertEqual(len(results), 1) + self.assertIsNone(results[0].error) + self.assertDictEqual(results[0].result, MockOperator.result) + doc = self.svc.get(doc_id=doc.id) self.assertEqual(doc.run_state, ExecutionRunState.COMPLETED) except: From c5500b213ca70981d93d6423f670112d3ad07969 Mon Sep 17 00:00:00 2001 From: Jacob Marks Date: Thu, 17 Oct 2024 10:53:59 -0400 Subject: [PATCH 38/69] add depth anything models to integration docs --- docs/source/integrations/huggingface.rst | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/source/integrations/huggingface.rst b/docs/source/integrations/huggingface.rst index e7fd8851d0..b7aaccda7e 100644 --- a/docs/source/integrations/huggingface.rst +++ b/docs/source/integrations/huggingface.rst @@ -402,6 +402,15 @@ method: from transformers import GLPNForDepthEstimation model = GLPNForDepthEstimation.from_pretrained("vinvino02/glpn-kitti") + # Depth Anything + from transformers import AutoModelForDepthEstimation + model = AutoModelForDepthEstimation.from_pretrained("LiheYoung/depth-anything-small-hf") + + # Depth Anything-V2 + from transformers import AutoModelForDepthEstimation + model = AutoModelForDepthEstimation.from_pretrained("depth-anything/Depth-Anything-V2-Small-hf") + + .. code-block:: python :linenos: From 7a7dfdd4b7edb6bccf8d4661cc719a74beeb1181 Mon Sep 17 00:00:00 2001 From: Camron Staley <55006027+CamronStaley@users.noreply.github.com> Date: Thu, 17 Oct 2024 13:08:18 -0500 Subject: [PATCH 39/69] Scheduler/default scheduled (#4871) * Add internal util for checking if remote or local * Changing default DO status based on internal or remote * updating scheduled_at or queued_at based on if it's internal * change util to use method that combines all 3 service checks into one * remove is internal service method since it's not used in OSS * update unit tests * update tests * reverse scheduled and queued * update comment * fix merge issues * fix merge * [skip ci] add test for is remote * fix test --------- Co-authored-by: Stuart Wheaton --- fiftyone/core/cli.py | 3 -- fiftyone/factory/repos/delegated_operation.py | 5 +-- .../factory/repos/delegated_operation_doc.py | 11 +++--- fiftyone/internal/__init__.py | 1 + fiftyone/internal/util.py | 34 +++++++++++++++++++ tests/unittests/operators/delegated_tests.py | 23 +++++++++++++ 6 files changed, 68 insertions(+), 9 deletions(-) create mode 100644 fiftyone/internal/util.py diff --git a/fiftyone/core/cli.py b/fiftyone/core/cli.py index 85912a875c..5a7aa889bc 100644 --- a/fiftyone/core/cli.py +++ b/fiftyone/core/cli.py @@ -7,7 +7,6 @@ """ import argparse -import warnings from collections import defaultdict from datetime import datetime import json @@ -19,8 +18,6 @@ import argcomplete from bson import ObjectId -import humanize -import pytz from tabulate import tabulate import webbrowser diff --git a/fiftyone/factory/repos/delegated_operation.py b/fiftyone/factory/repos/delegated_operation.py index 3355792c97..ac2a9c20c1 100644 --- a/fiftyone/factory/repos/delegated_operation.py +++ b/fiftyone/factory/repos/delegated_operation.py @@ -15,6 +15,7 @@ from pymongo import IndexModel from pymongo.collection import Collection +from fiftyone.internal.util import is_remote_service from fiftyone.factory import DelegatedOperationPagingParams from fiftyone.factory.repos import DelegatedOperationDocument from fiftyone.operators.executor import ( @@ -134,7 +135,7 @@ def __init__(self, collection: Collection = None): self._collection = ( collection if collection is not None else self._get_collection() ) - + self.is_remote = is_remote_service() self._create_indexes() def _get_collection(self) -> Collection: @@ -170,7 +171,7 @@ def _create_indexes(self): self._collection.create_indexes(indices_to_create) def queue_operation(self, **kwargs: Any) -> DelegatedOperationDocument: - op = DelegatedOperationDocument() + op = DelegatedOperationDocument(is_remote=self.is_remote) for prop in self.required_props: if prop not in kwargs: raise ValueError("Missing required property '%s'" % prop) diff --git a/fiftyone/factory/repos/delegated_operation_doc.py b/fiftyone/factory/repos/delegated_operation_doc.py index 150b8a05de..1360783ad3 100644 --- a/fiftyone/factory/repos/delegated_operation_doc.py +++ b/fiftyone/factory/repos/delegated_operation_doc.py @@ -25,6 +25,7 @@ def __init__( operator: str = None, delegation_target: str = None, context: dict = None, + is_remote: bool = False, ): self.operator = operator self.label = None @@ -35,10 +36,12 @@ def __init__( else context ) self.run_state = ( - ExecutionRunState.QUEUED - ) # default to queued state on create + ExecutionRunState.SCHEDULED + if is_remote + else ExecutionRunState.QUEUED + ) # if running locally use QUEUED otherwise SCHEDULED self.run_link = None - self.queued_at = datetime.utcnow() + self.queued_at = datetime.utcnow() if not is_remote else None self.updated_at = datetime.utcnow() self.status = None self.dataset_id = None @@ -46,7 +49,7 @@ def __init__( self.pinned = False self.completed_at = None self.failed_at = None - self.scheduled_at = None + self.scheduled_at = datetime.utcnow() if is_remote else None self.result = None self.id = None self._doc = None diff --git a/fiftyone/internal/__init__.py b/fiftyone/internal/__init__.py index 948c11f1b2..8d5a4915b3 100644 --- a/fiftyone/internal/__init__.py +++ b/fiftyone/internal/__init__.py @@ -7,3 +7,4 @@ """ from .secrets import * +from .util import is_remote_service diff --git a/fiftyone/internal/util.py b/fiftyone/internal/util.py new file mode 100644 index 0000000000..bfde1b3db6 --- /dev/null +++ b/fiftyone/internal/util.py @@ -0,0 +1,34 @@ +""" +FiftyOne internal utilities. + +| Copyright 2017-2024, Voxel51, Inc. +| `voxel51.com `_ +| +""" + + +def is_remote_service(): + """Whether the SDK is running in a remote service context. + + Returns: + True/False + """ + return has_encryption_key() and has_api_key() + + +def has_encryption_key(): + """Whether the current environment has an encryption key. + + Returns: + True/False + """ + return False + + +def has_api_key(): + """Whether the current environment has an API key. + + Returns: + True/False + """ + return False diff --git a/tests/unittests/operators/delegated_tests.py b/tests/unittests/operators/delegated_tests.py index ef48c78ee9..4939124942 100644 --- a/tests/unittests/operators/delegated_tests.py +++ b/tests/unittests/operators/delegated_tests.py @@ -28,6 +28,7 @@ ExecutionResult, ExecutionRunState, ) +from fiftyone.factory.repos import delegated_operation from fiftyone.operators.operator import Operator, OperatorConfig @@ -1333,3 +1334,25 @@ async def test_set_completed_in_async_context( doc = self.svc.set_completed(doc_id=doc.id) self.assertEqual(doc.run_state, ExecutionRunState.COMPLETED) + + @patch.object( + delegated_operation, + "is_remote_service", + return_value=True, + ) + def test_queue_op_remote_service( + self, mock_is_remote_service, mock_get_operator, mock_operator_exists + ): + db = delegated_operation.MongoDelegatedOperationRepo() + dos = DelegatedOperationService(repo=db) + ctx = ExecutionContext() + ctx.request_params = {"foo": "bar"} + doc = dos.queue_operation( + operator="@voxelfiftyone/operator/foo", + label=mock_get_operator.return_value.name, + delegation_target="test_target", + context=ctx.serialize(), + ) + self.docs_to_delete.append(doc) + self.assertTrue(db.is_remote) + self.assertEqual(doc.run_state, ExecutionRunState.SCHEDULED) From 7cf9d3c017de3dd67a0e6fd076672a1297339ffe Mon Sep 17 00:00:00 2001 From: Kacey Date: Thu, 17 Oct 2024 20:18:09 -0700 Subject: [PATCH 40/69] rm extra await (#4945) --- app/packages/state/src/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/packages/state/src/utils.ts b/app/packages/state/src/utils.ts index 4b61bc53f4..400567814c 100644 --- a/app/packages/state/src/utils.ts +++ b/app/packages/state/src/utils.ts @@ -172,7 +172,7 @@ const fetchRelay: FetchFunction = async ( params, variables ): Promise => { - const data = await await getFetchFunction()( + const data = await getFetchFunction()( "POST", "/graphql", { From cdc587d14d966547c3fc1961d57adde9b7f43c85 Mon Sep 17 00:00:00 2001 From: Br2850 Date: Fri, 18 Oct 2024 10:25:48 -0600 Subject: [PATCH 41/69] simplify --- app/packages/operators/src/CustomPanel.tsx | 10 ++++------ app/packages/operators/src/useCustomPanelHooks.ts | 5 ----- app/packages/spaces/src/hooks.ts | 1 + 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/app/packages/operators/src/CustomPanel.tsx b/app/packages/operators/src/CustomPanel.tsx index dcbd671fce..e463a29300 100644 --- a/app/packages/operators/src/CustomPanel.tsx +++ b/app/packages/operators/src/CustomPanel.tsx @@ -12,11 +12,8 @@ import OperatorIO from "./OperatorIO"; import { PANEL_LOAD_TIMEOUT } from "./constants"; import { useActivePanelEventsCount } from "./hooks"; import { Property } from "./types"; -import { - CustomPanelProps, - useCustomPanelHooks, - trackPanelClose, -} from "./useCustomPanelHooks"; +import { CustomPanelProps, useCustomPanelHooks } from "./useCustomPanelHooks"; +import { useTrackEvent } from "@fiftyone/analytics"; export function CustomPanel(props: CustomPanelProps) { const { panelId, dimensions, panelName, panelLabel } = props; @@ -33,11 +30,12 @@ export function CustomPanel(props: CustomPanelProps) { } = useCustomPanelHooks(props); const pending = fos.useTimeout(PANEL_LOAD_TIMEOUT); const setPanelCloseEffect = useSetPanelCloseEffect(); + const trackEvent = useTrackEvent(); useEffect(() => { setPanelCloseEffect(() => { clearUseKeyStores(panelId); - trackPanelClose(panelName); + trackEvent("close_panel", { panel: panelName }); }); }, []); diff --git a/app/packages/operators/src/useCustomPanelHooks.ts b/app/packages/operators/src/useCustomPanelHooks.ts index 95ae2a9cd1..feeb93ff66 100644 --- a/app/packages/operators/src/useCustomPanelHooks.ts +++ b/app/packages/operators/src/useCustomPanelHooks.ts @@ -224,8 +224,3 @@ function getPanelViewData(panelState) { const data = panelState?.data; return merge({}, { ...state }, { ...data }); } - -export function trackPanelClose(panelName) { - const trackEvent = useTrackEvent(); - return trackEvent("close_panel", { panel_name: panelName }); -} diff --git a/app/packages/spaces/src/hooks.ts b/app/packages/spaces/src/hooks.ts index 3d9eaafb4f..d56e904aa6 100644 --- a/app/packages/spaces/src/hooks.ts +++ b/app/packages/spaces/src/hooks.ts @@ -40,6 +40,7 @@ import { SpaceNodeType, } from "./types"; import { getNodes } from "./utils"; +import { useTrackEvent } from "@fiftyone/analytics"; export function useSpaces(id: string, defaultState?: SpaceNodeJSON) { const [state, setState] = useRecoilState(spaceSelector(id)); From 01764ea28b575d6a22501c1f414890a668b106bc Mon Sep 17 00:00:00 2001 From: Br2850 Date: Fri, 18 Oct 2024 10:27:33 -0600 Subject: [PATCH 42/69] remove unused imports --- app/packages/operators/src/useCustomPanelHooks.ts | 1 - app/packages/spaces/src/hooks.ts | 1 - 2 files changed, 2 deletions(-) diff --git a/app/packages/operators/src/useCustomPanelHooks.ts b/app/packages/operators/src/useCustomPanelHooks.ts index feeb93ff66..afbaf851dd 100644 --- a/app/packages/operators/src/useCustomPanelHooks.ts +++ b/app/packages/operators/src/useCustomPanelHooks.ts @@ -1,4 +1,3 @@ -import { useTrackEvent } from "@fiftyone/analytics"; import { debounce, merge } from "lodash"; import { useCallback, useEffect, useMemo } from "react"; diff --git a/app/packages/spaces/src/hooks.ts b/app/packages/spaces/src/hooks.ts index d56e904aa6..3d9eaafb4f 100644 --- a/app/packages/spaces/src/hooks.ts +++ b/app/packages/spaces/src/hooks.ts @@ -40,7 +40,6 @@ import { SpaceNodeType, } from "./types"; import { getNodes } from "./utils"; -import { useTrackEvent } from "@fiftyone/analytics"; export function useSpaces(id: string, defaultState?: SpaceNodeJSON) { const [state, setState] = useRecoilState(spaceSelector(id)); From adad5690407db244610f1e1dccbe8085a9cf00c0 Mon Sep 17 00:00:00 2001 From: Sashank Aryal <66688606+sashankaryal@users.noreply.github.com> Date: Sat, 19 Oct 2024 14:00:08 +0545 Subject: [PATCH 43/69] fix ImaVid hover bug: pass correct modal param flag (#4938) --- .../DynamicGroup/carousel/DynamicGroupsFlashlightWrapper.tsx | 2 +- app/packages/core/src/components/Modal/Group/GroupCarousel.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/packages/core/src/components/Modal/Group/DynamicGroup/carousel/DynamicGroupsFlashlightWrapper.tsx b/app/packages/core/src/components/Modal/Group/DynamicGroup/carousel/DynamicGroupsFlashlightWrapper.tsx index a27625d224..b1e98b5dce 100644 --- a/app/packages/core/src/components/Modal/Group/DynamicGroup/carousel/DynamicGroupsFlashlightWrapper.tsx +++ b/app/packages/core/src/components/Modal/Group/DynamicGroup/carousel/DynamicGroupsFlashlightWrapper.tsx @@ -59,7 +59,7 @@ export const DynamicGroupsFlashlightWrapper = React.memo(() => { ); const createLooker = fos.useCreateLooker( - false, + true, true, { ...opts, diff --git a/app/packages/core/src/components/Modal/Group/GroupCarousel.tsx b/app/packages/core/src/components/Modal/Group/GroupCarousel.tsx index b3846a75c7..999ec709a5 100644 --- a/app/packages/core/src/components/Modal/Group/GroupCarousel.tsx +++ b/app/packages/core/src/components/Modal/Group/GroupCarousel.tsx @@ -72,7 +72,7 @@ const Column: React.FC = () => { ); const createLooker = fos.useCreateLooker( - false, + true, true, { ...opts, From e42b7069ef4b124e64a1f72150a3ae8c1925d24f Mon Sep 17 00:00:00 2001 From: Hieu Trinh <156294040+hieutomra@users.noreply.github.com> Date: Sun, 20 Oct 2024 12:30:00 +1300 Subject: [PATCH 44/69] Add referrerPolicy fetch.ts (#4944) Set referrerPolicy to 'same-origin' rather default `strict-origin-when-cross-origin` The strict referrer break fiftyone app when behind reverse proxy --- app/packages/utilities/src/fetch.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/packages/utilities/src/fetch.ts b/app/packages/utilities/src/fetch.ts index 001709bcbd..0ee061bc93 100644 --- a/app/packages/utilities/src/fetch.ts +++ b/app/packages/utilities/src/fetch.ts @@ -129,6 +129,7 @@ export const setFetchFunction = ( mode: "cors", body: body ? JSON.stringify(body) : null, signal: controller.signal, + referrerPolicy: 'same-origin', }); if (response.status >= 400) { @@ -271,6 +272,7 @@ export const getEventSource = ( method: "POST", signal, body: JSON.stringify(body), + referrerPolicy: 'same-origin', async onopen(response) { if (response.ok) { events.onopen && events.onopen(); From b01dd09a79c0c70d6c00f725001b8cd048f84f53 Mon Sep 17 00:00:00 2001 From: imanjra Date: Mon, 7 Oct 2024 00:15:45 -0400 Subject: [PATCH 45/69] add spaces context to operators --- app/packages/operators/src/CustomPanel.tsx | 2 ++ app/packages/operators/src/hooks.ts | 6 ++++ app/packages/operators/src/operators.ts | 21 +++++++++++- app/packages/operators/src/state.ts | 10 ++++++ .../operators/src/useCustomPanelHooks.ts | 9 +++++ docs/source/plugins/developing_plugins.rst | 14 ++++++++ fiftyone/operators/builtin.py | 34 +++++-------------- fiftyone/operators/executor.py | 12 +++++++ fiftyone/operators/operations.py | 1 + fiftyone/operators/panel.py | 1 + 10 files changed, 83 insertions(+), 27 deletions(-) diff --git a/app/packages/operators/src/CustomPanel.tsx b/app/packages/operators/src/CustomPanel.tsx index cf9f82a5b3..ae7caa88de 100644 --- a/app/packages/operators/src/CustomPanel.tsx +++ b/app/packages/operators/src/CustomPanel.tsx @@ -115,6 +115,7 @@ export function defineCustomPanel({ on_change_selected_labels, on_change_extended_selection, on_change_group_slice, + on_change_spaces, panel_name, panel_label, }) { @@ -132,6 +133,7 @@ export function defineCustomPanel({ onChangeSelectedLabels={on_change_selected_labels} onChangeExtendedSelection={on_change_extended_selection} onChangeGroupSlice={on_change_group_slice} + onChangeSpaces={on_change_spaces} 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 a636f1b263..855c6a2b1c 100644 --- a/app/packages/operators/src/hooks.ts +++ b/app/packages/operators/src/hooks.ts @@ -28,6 +28,8 @@ function useOperatorThrottledContextSetter() { const groupSlice = useRecoilValue(fos.groupSlice); const currentSample = useCurrentSample(); const setContext = useSetRecoilState(operatorThrottledContext); + const spaces = useRecoilValue(fos.sessionSpaces); + const workspaceName = spaces._name; const setThrottledContext = useMemo(() => { return debounce( (context) => { @@ -49,6 +51,8 @@ function useOperatorThrottledContextSetter() { currentSample, viewName, groupSlice, + spaces, + workspaceName, }); }, [ setThrottledContext, @@ -61,6 +65,8 @@ function useOperatorThrottledContextSetter() { currentSample, viewName, groupSlice, + spaces, + workspaceName, ]); } diff --git a/app/packages/operators/src/operators.ts b/app/packages/operators/src/operators.ts index d16f175fba..a96f505406 100644 --- a/app/packages/operators/src/operators.ts +++ b/app/packages/operators/src/operators.ts @@ -1,5 +1,6 @@ import { AnalyticsInfo, usingAnalytics } from "@fiftyone/analytics"; -import { ServerError, getFetchFunction, isNullish } from "@fiftyone/utilities"; +import { SpaceNode, spaceNodeFromJSON, SpaceNodeJSON } from "@fiftyone/spaces"; +import { getFetchFunction, isNullish, ServerError } from "@fiftyone/utilities"; import { CallbackInterface } from "recoil"; import { QueueItemStatus } from "./constants"; import * as types from "./types"; @@ -92,6 +93,8 @@ export type RawContext = { }; groupSlice: string; queryPerformance?: boolean; + spaces: SpaceNodeJSON; + workspaceName: string; }; export class ExecutionContext { @@ -140,6 +143,12 @@ export class ExecutionContext { public get queryPerformance(): boolean { return Boolean(this._currentContext.queryPerformance); } + public get spaces(): SpaceNode { + return spaceNodeFromJSON(this._currentContext.spaces); + } + public get workspaceName(): string { + return this._currentContext.workspaceName; + } getCurrentPanelId(): string | null { return this.params.panel_id || this.currentPanel?.id || null; @@ -548,6 +557,8 @@ async function executeOperatorAsGenerator( view: currentContext.view, view_name: currentContext.viewName, group_slice: currentContext.groupSlice, + spaces: currentContext.spaces, + workspace_name: currentContext.workspaceName, }, "json-stream" ); @@ -712,6 +723,8 @@ export async function executeOperatorWithContext( view_name: currentContext.viewName, group_slice: currentContext.groupSlice, query_performance: currentContext.queryPerformance, + spaces: currentContext.spaces, + workspace_name: currentContext.workspaceName, } ); result = serverResult.result; @@ -815,6 +828,8 @@ export async function resolveRemoteType( view: currentContext.view, view_name: currentContext.viewName, group_slice: currentContext.groupSlice, + spaces: currentContext.spaces, + workspace_name: currentContext.workspaceName, } ); @@ -889,6 +904,8 @@ export async function resolveExecutionOptions( view: currentContext.view, view_name: currentContext.viewName, group_slice: currentContext.groupSlice, + spaces: currentContext.spaces, + workspace_name: currentContext.workspaceName, } ); @@ -920,6 +937,8 @@ export async function fetchRemotePlacements(ctx: ExecutionContext) { current_sample: currentContext.currentSample, view_name: currentContext.viewName, group_slice: currentContext.groupSlice, + spaces: currentContext.spaces, + workspace_name: currentContext.workspaceName, } ); if (result && result.error) { diff --git a/app/packages/operators/src/state.ts b/app/packages/operators/src/state.ts index cf0f08cd14..535f61c0c2 100644 --- a/app/packages/operators/src/state.ts +++ b/app/packages/operators/src/state.ts @@ -95,6 +95,8 @@ const globalContextSelector = selector({ const extendedSelection = get(fos.extendedSelection); const groupSlice = get(fos.groupSlice); const queryPerformance = typeof get(fos.lightningThreshold) === "number"; + const spaces = get(fos.sessionSpaces); + const workspaceName = spaces?._name; return { datasetName, @@ -107,6 +109,8 @@ const globalContextSelector = selector({ extendedSelection, groupSlice, queryPerformance, + spaces, + workspaceName, }; }, }); @@ -148,6 +152,8 @@ const useExecutionContext = (operatorName, hooks = {}) => { extendedSelection, groupSlice, queryPerformance, + spaces, + workspaceName, } = curCtx; const [analyticsInfo] = useAnalyticsInfo(); const ctx = useMemo(() => { @@ -166,6 +172,8 @@ const useExecutionContext = (operatorName, hooks = {}) => { analyticsInfo, groupSlice, queryPerformance, + spaces, + workspaceName, }, hooks ); @@ -182,6 +190,8 @@ const useExecutionContext = (operatorName, hooks = {}) => { currentSample, groupSlice, queryPerformance, + spaces, + workspaceName, ]); return ctx; diff --git a/app/packages/operators/src/useCustomPanelHooks.ts b/app/packages/operators/src/useCustomPanelHooks.ts index afbaf851dd..155f67b98c 100644 --- a/app/packages/operators/src/useCustomPanelHooks.ts +++ b/app/packages/operators/src/useCustomPanelHooks.ts @@ -26,6 +26,8 @@ export interface CustomPanelProps { onChangeSelectedLabels?: string; onChangeExtendedSelection?: string; onChangeGroupSlice?: string; + onChangeSpaces?: string; + onChangeWorkspace?: string; dimensions: DimensionsType | null; panelName?: string; panelLabel?: string; @@ -136,6 +138,13 @@ export function useCustomPanelHooks(props: CustomPanelProps): CustomPanelHooks { ctx.groupSlice, props.onChangeGroupSlice ); + useCtxChangePanelEvent(isLoaded, panelId, ctx.spaces, props.onChangeSpaces); + useCtxChangePanelEvent( + isLoaded, + panelId, + ctx.workspaceName, + props.onChangeWorkspace + ); useEffect(() => { onLoad(); diff --git a/docs/source/plugins/developing_plugins.rst b/docs/source/plugins/developing_plugins.rst index 68b9a1c12a..d0769a5d18 100644 --- a/docs/source/plugins/developing_plugins.rst +++ b/docs/source/plugins/developing_plugins.rst @@ -1001,6 +1001,7 @@ contains the following properties: if any - `ctx.results` - a dict containing the outputs of the `execute()` method, if it has been called +- `ctx.spaces` - The current workspace or the state of spaces in the app. - `ctx.hooks` **(JS only)** - the return value of the operator's `useHooks()` method @@ -2060,6 +2061,19 @@ subsequent sections. ctx.panel.set_state("event", "on_change_group_slice") ctx.panel.set_data("event_data", event) + def on_change_spaces(self, ctx): + """Implement this method to set panel state/data when the current + state of spaces changes in the app. + + The current state of spaces will be available via ``ctx.spaces``. + """ + event = { + "data": ctx.spaces, + "description": "the current state of spaces", + } + ctx.panel.set_state("event", "on_change_spaces") + ctx.panel.set_data("event_data", event) + ####################################################################### # Custom events # These events are defined by user code above and, just like builtin diff --git a/fiftyone/operators/builtin.py b/fiftyone/operators/builtin.py index 12d4ee76bb..c0f163f45d 100644 --- a/fiftyone/operators/builtin.py +++ b/fiftyone/operators/builtin.py @@ -1939,28 +1939,6 @@ def resolve_input(self, ctx): ), ) - # @todo infer this automatically from current App spaces - spaces_prop = inputs.oneof( - "spaces", - [types.String(), types.Object()], - default=None, - required=True, - label="Spaces", - description=( - "JSON description of the workspace to save: " - "`print(session.spaces.to_json(True))`" - ), - view=types.CodeView(), - ) - - spaces = ctx.params.get("spaces", None) - if spaces is not None: - try: - _parse_spaces(spaces) - except: - spaces_prop.invalid = True - spaces_prop.error_message = "Invalid workspace definition" - name = ctx.params.get("name", None) if name in workspaces: @@ -1979,7 +1957,10 @@ def execute(self, ctx): color = ctx.params.get("color", None) spaces = ctx.params.get("spaces", None) - spaces = _parse_spaces(spaces) + if spaces is None: + spaces = ctx.spaces + else: + spaces = _parse_spaces(spaces) ctx.dataset.save_workspace( name, @@ -2034,11 +2015,12 @@ def _edit_workspace_info_inputs(ctx, inputs): for key in workspaces: workspace_selector.add_choice(key, label=key) - # @todo default to current workspace name, if one is currently open + current_workspace = ctx.spaces.name if ctx.spaces else None inputs.enum( "name", workspace_selector.values(), required=True, + default=current_workspace, label="Workspace", description="The workspace to edit", view=workspace_selector, @@ -2102,11 +2084,11 @@ def resolve_input(self, ctx): workspace_selector = types.AutocompleteView() for key in workspaces: workspace_selector.add_choice(key, label=key) - + current_workspace = ctx.spaces.name if ctx.spaces else None inputs.enum( "name", workspace_selector.values(), - default=None, + default=current_workspace, required=True, label="Workspace", description="The workspace to delete", diff --git a/fiftyone/operators/executor.py b/fiftyone/operators/executor.py index e0e6ac784c..7624653a14 100644 --- a/fiftyone/operators/executor.py +++ b/fiftyone/operators/executor.py @@ -718,6 +718,18 @@ def query_performance(self): """Whether query performance is enabled.""" return self.request_params.get("query_performance", None) + @property + def spaces(self): + """The current workspace or the state of spaces in the app.""" + workspace_name = self.request_params.get("workspace_name", None) + if workspace_name is not None: + return self.dataset.load_workspace(workspace_name) + + spaces_dict = self.request_params.get("spaces", None) + if spaces_dict is None: + return None + return fo.Space.from_dict(spaces_dict) + def prompt( self, operator_uri, diff --git a/fiftyone/operators/operations.py b/fiftyone/operators/operations.py index 933f6f3d55..84c5e78e90 100644 --- a/fiftyone/operators/operations.py +++ b/fiftyone/operators/operations.py @@ -356,6 +356,7 @@ def register_panel( current extended selection changes on_change_group_slice (None): an operator to invoke when the group slice changes + on_change_spaces (None): an operator to invoke when the current spaces changes allow_duplicates (False): whether to allow multiple instances of the panel to the opened """ diff --git a/fiftyone/operators/panel.py b/fiftyone/operators/panel.py index b9d897c2fd..1a286e044d 100644 --- a/fiftyone/operators/panel.py +++ b/fiftyone/operators/panel.py @@ -126,6 +126,7 @@ def on_startup(self, ctx): "on_change_selected_labels", "on_change_extended_selection", "on_change_group_slice", + "on_change_spaces", ] for method in methods + ctx_change_events: if hasattr(self, method) and callable(getattr(self, method)): From 385e5471b3e7bffdba540d4eb475bbef0759888d Mon Sep 17 00:00:00 2001 From: imanjra Date: Tue, 17 Sep 2024 12:43:18 -0400 Subject: [PATCH 46/69] TableView enhancements --- .../SchemaIO/components/ActionsMenu.tsx | 108 ++++++++++ .../plugins/SchemaIO/components/TableView.tsx | 184 ++++++++++++++++-- fiftyone/operators/types.py | 32 +++ 3 files changed, 303 insertions(+), 21 deletions(-) create mode 100644 app/packages/core/src/plugins/SchemaIO/components/ActionsMenu.tsx diff --git a/app/packages/core/src/plugins/SchemaIO/components/ActionsMenu.tsx b/app/packages/core/src/plugins/SchemaIO/components/ActionsMenu.tsx new file mode 100644 index 0000000000..86ea87ac65 --- /dev/null +++ b/app/packages/core/src/plugins/SchemaIO/components/ActionsMenu.tsx @@ -0,0 +1,108 @@ +import { MuiIconFont } from "@fiftyone/components"; +import { MoreVert } from "@mui/icons-material"; +import { + Box, + Button, + IconButton, + ListItemIcon, + ListItemText, + Menu, + MenuItem, + Stack, +} from "@mui/material"; +import React, { useCallback } from "react"; + +const DEFAULT_MAX_INLINE = 1; + +export default function ActionsMenu(props: ActionsPropsType) { + const { actions, maxInline = DEFAULT_MAX_INLINE } = props; + + if (actions.length === maxInline) { + return ( + + {actions.map((action) => ( + + ))} + + ); + } + + return ; +} + +function ActionsOverflowMenu(props: ActionsPropsType) { + const { actions } = props; + const [open, setOpen] = React.useState(false); + const anchorRef = React.useRef(null); + + const handleClose = useCallback(() => { + setOpen(false); + }, []); + + return ( + + { + setOpen(!open); + }} + ref={anchorRef} + > + + + + {actions.map((action) => { + const { name, onClick } = action; + return ( + { + handleClose(); + onClick?.(action, e); + }} + /> + ); + })} + + + ); +} + +function Action(props: ActionPropsType) { + const { label, name, onClick, icon, variant, mode } = props; + + const Icon = icon ? : null; + + const handleClick = useCallback( + (e: React.MouseEvent) => { + onClick?.(props, e); + }, + [onClick, props] + ); + + return mode === "inline" ? ( + + ) : ( + + {Icon && {Icon}} + {label || name} + + ); +} + +type ActionsPropsType = { + actions: Array; + maxInline?: number; +}; + +type ActionPropsType = { + name: string; + label: string; + onClick: (action: ActionPropsType, e: React.MouseEvent) => void; + icon: string; + variant: string; + mode: "inline" | "menu"; +}; diff --git a/app/packages/core/src/plugins/SchemaIO/components/TableView.tsx b/app/packages/core/src/plugins/SchemaIO/components/TableView.tsx index 100cb1b28e..abee3c1ed9 100644 --- a/app/packages/core/src/plugins/SchemaIO/components/TableView.tsx +++ b/app/packages/core/src/plugins/SchemaIO/components/TableView.tsx @@ -1,3 +1,6 @@ +import { scrollable } from "@fiftyone/components"; +import { usePanelEvent } from "@fiftyone/operators"; +import { usePanelId } from "@fiftyone/spaces"; import { Box, Paper, @@ -8,23 +11,73 @@ import { TableHead, TableRow, } from "@mui/material"; -import React from "react"; +import { isPlainObject } from "lodash"; +import React, { useCallback } from "react"; import { HeaderView } from "."; -import EmptyState from "./EmptyState"; import { getComponentProps } from "../utils"; +import { ViewPropsType } from "../utils/types"; +import ActionsMenu from "./ActionsMenu"; +import EmptyState from "./EmptyState"; -export default function TableView(props) { - const { schema, data } = props; - const { view = {}, default: defaultValue } = schema; - const { columns } = view; +export default function TableView(props: ViewPropsType) { + const { path, schema } = props; + const { view = {} } = schema; + const { + columns, + row_actions = [], + on_click_cell, + on_click_row, + on_click_column, + actions_label, + } = view; + const { rows, selectedCells, selectedRows, selectedColumns } = + getTableData(props); + const dataMissing = rows.length === 0; + const hasRowActions = row_actions.length > 0; + const panelId = usePanelId(); + const handleClick = usePanelEvent(); - const table = Array.isArray(data) - ? data - : Array.isArray(defaultValue) - ? defaultValue - : []; + const getRowActions = useCallback((row) => { + const computedRowActions = [] as any; + for (const action of row_actions) { + if (action.rows?.[row] !== false) { + computedRowActions.push({ + ...action, + onClick: (action, e) => { + handleClick(panelId, { + operator: action.on_click, + params: { path, event: action.name, row }, + }); + }, + }); + } + } + return computedRowActions; + }, []); - const dataMissing = table.length === 0; + const handleCellClick = useCallback( + (row, column) => { + if (on_click_cell) { + handleClick(panelId, { + operator: on_click_cell, + params: { row, column, path, event: "on_click_cell" }, + }); + } + if (on_click_row) { + handleClick(panelId, { + operator: on_click_row, + params: { row, path, event: "on_click_row" }, + }); + } + if (on_click_column) { + handleClick(panelId, { + operator: on_click_column, + params: { column, path, event: "on_click_column" }, + }); + } + }, + [on_click_cell, on_click_row, on_click_column, handleClick, panelId, path] + ); return ( @@ -33,37 +86,73 @@ export default function TableView(props) { {!dataMissing && ( - {columns.map(({ key, label }) => ( + {columns.map(({ key, label }, columnIndex) => ( { + handleCellClick(-1, columnIndex); + }} {...getComponentProps(props, "tableHeadCell")} > {label} ))} + {hasRowActions && ( + { + handleCellClick(-1, -1); + }} + > + {actions_label || "Actions"} + + )} - {table.map((item) => ( + {rows.map((item, rowIndex) => ( - {columns.map(({ key }) => ( - - {item[key]} + {columns.map(({ key }, columnIndex) => { + const coordinate = [rowIndex, columnIndex].join(","); + const isSelected = + selectedCells.has(coordinate) || + selectedRows.has(rowIndex) || + selectedColumns.has(columnIndex); + return ( + { + handleCellClick(rowIndex, columnIndex); + }} + {...getComponentProps(props, "tableBodyCell")} + > + {formatCellValue(item[key], props)} + + ); + })} + {hasRowActions && ( + + - ))} + )} ))} @@ -73,3 +162,56 @@ export default function TableView(props) { ); } + +function getTableData(props) { + const { schema, data } = props; + const defaultValue = schema?.default; + + if (isAdvancedData(data)) { + return parseAdvancedData(data); + } + if (isAdvancedData(defaultValue)) { + return parseAdvancedData(defaultValue); + } + return { + rows: Array.isArray(data) + ? data + : Array.isArray(defaultValue) + ? defaultValue + : [], + }; +} + +function isAdvancedData(data) { + return ( + isPlainObject(data) && + Array.isArray(data?.rows) && + Array.isArray(data?.columns) + ); +} + +function parseAdvancedData(data) { + const rows = data.rows.map((row) => { + return data.columns.reduce((cells, column, cellIndex) => { + cells[column] = row[cellIndex]; + return cells; + }, {}); + }); + const selectedCellsRaw = data?.selectedCells || data?.selected_cells || []; + const selectedRowsRaw = data?.selectedRows || data?.selected_rows || []; + const selectedColumnsRaw = + data?.selectedColumns || data?.selected_columns || []; + const selectedCells = new Set(selectedCellsRaw.map((cell) => cell.join(","))); + const selectedRows = new Set(selectedRowsRaw); + const selectedColumns = new Set(selectedColumnsRaw); + return { rows, selectedCells, selectedRows, selectedColumns }; +} + +function formatCellValue(value: string, props: ViewPropsType) { + const round = props?.schema?.view?.round; + const valueAsFloat = parseFloat(value); + if (!Number.isNaN(valueAsFloat) && typeof round === "number") { + return valueAsFloat.toFixed(round); + } + return value; +} diff --git a/fiftyone/operators/types.py b/fiftyone/operators/types.py index a22bd01230..f861433eeb 100644 --- a/fiftyone/operators/types.py +++ b/fiftyone/operators/types.py @@ -1618,16 +1618,39 @@ def to_json(self): return {**super().to_json(), "key": self.key} +class Action(View): + """An action (currently supported only in a :class:`TableView`). + + Args: + name: the name of the action + label (None): the label of the action + icon (None): the icon of the action + on_click: the operator to execute when the action is clicked + """ + + def __init__(self, **kwargs): + super().__init__(**kwargs) + + def clone(self): + clone = Action(**self._kwargs) + return clone + + def to_json(self): + return {**super().to_json()} + + class TableView(View): """Displays a table. Args: columns (None): a list of :class:`Column` objects to display + row_actions (None): a list of :class:`Action` objects to display """ def __init__(self, **kwargs): super().__init__(**kwargs) self.columns = kwargs.get("columns", []) + self.row_actions = kwargs.get("row_actions", []) def keys(self): return [column.key for column in self.columns] @@ -1637,15 +1660,24 @@ def add_column(self, key, **kwargs): self.columns.append(column) return column + def add_row_action(self, name, on_click, label=None, icon=None, **kwargs): + row_action = Action( + name=name, on_click=on_click, label=label, icon=icon, **kwargs + ) + self.row_actions.append(row_action) + return row_action + def clone(self): clone = super().clone() clone.columns = [column.clone() for column in self.columns] + clone.row_actions = [action.clone() for action in self.row_actions] return clone def to_json(self): return { **super().to_json(), "columns": [column.to_json() for column in self.columns], + "row_actions": [action.to_json() for action in self.row_actions], } From 8f6cb6673c124ff7e642c6e6449cde5b0f01f27b Mon Sep 17 00:00:00 2001 From: Sashank Aryal <66688606+sashankaryal@users.noreply.github.com> Date: Mon, 21 Oct 2024 22:18:58 +0545 Subject: [PATCH 47/69] link docs to new hello world js plugin (#4952) --- docs/source/plugins/developing_plugins.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/source/plugins/developing_plugins.rst b/docs/source/plugins/developing_plugins.rst index 7083a06c4b..1ee0f0e643 100644 --- a/docs/source/plugins/developing_plugins.rst +++ b/docs/source/plugins/developing_plugins.rst @@ -142,8 +142,8 @@ directory. .. note:: For vite configs we recommend forking the - `FiftyOne Plugins `_ repository - and following the conventions there to build your plugin. + `FiftyOne Plugins `_ + repository and following the conventions there to build your plugin. .. _plugin-anatomy: From 080e8932a1170878943bd0b981503ddcf299bd56 Mon Sep 17 00:00:00 2001 From: imanjra Date: Mon, 21 Oct 2024 10:27:09 -0400 Subject: [PATCH 48/69] fix TableView for backward compatibility --- .../core/src/plugins/SchemaIO/components/TableView.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/packages/core/src/plugins/SchemaIO/components/TableView.tsx b/app/packages/core/src/plugins/SchemaIO/components/TableView.tsx index abee3c1ed9..89747ad9ab 100644 --- a/app/packages/core/src/plugins/SchemaIO/components/TableView.tsx +++ b/app/packages/core/src/plugins/SchemaIO/components/TableView.tsx @@ -179,6 +179,9 @@ function getTableData(props) { : Array.isArray(defaultValue) ? defaultValue : [], + selectedCells: new Set(), + selectedRows: new Set(), + selectedColumns: new Set(), }; } From d3e87076dc604f4a0096602c5e4bfb094b1098d8 Mon Sep 17 00:00:00 2001 From: Minh-Tue Vo Date: Mon, 21 Oct 2024 13:11:26 -0700 Subject: [PATCH 49/69] Toast component (#4955) * Added Toast component * Addressed feedback --------- Co-authored-by: minhtuevo --- .../components/src/components/Toast/Toast.tsx | 41 +++++++++++++++++++ .../components/src/components/Toast/index.ts | 1 + .../components/src/components/index.ts | 1 + 3 files changed, 43 insertions(+) create mode 100644 app/packages/components/src/components/Toast/Toast.tsx create mode 100644 app/packages/components/src/components/Toast/index.ts diff --git a/app/packages/components/src/components/Toast/Toast.tsx b/app/packages/components/src/components/Toast/Toast.tsx new file mode 100644 index 0000000000..6cd18102eb --- /dev/null +++ b/app/packages/components/src/components/Toast/Toast.tsx @@ -0,0 +1,41 @@ +import React, { useState } from "react"; +import { Snackbar, Button, SnackbarContent } from "@mui/material"; +import Box from '@mui/material/Box'; +import Typography from '@mui/material/Typography'; +import BoltIcon from '@mui/icons-material/Bolt'; // Icon for the lightning bolt + +// Define types for the props +interface ToastProps { + action: React.ReactNode; // Accepts any valid React component, element, or JSX + message: React.ReactNode; // Accepts any valid React component, element, or JSX + duration?: number; // Optional duration, with a default value +} + +const Toast: React.FC = ({ action, message, duration = 5000 }) => { + const [open, setOpen] = useState(true); + + const handleClose = (event, reason) => { + if (reason === "clickaway") { + return; + } + setOpen(false); + }; + + return ( + + + + ); +} + +export default Toast; diff --git a/app/packages/components/src/components/Toast/index.ts b/app/packages/components/src/components/Toast/index.ts new file mode 100644 index 0000000000..43c586feaf --- /dev/null +++ b/app/packages/components/src/components/Toast/index.ts @@ -0,0 +1 @@ +export {default as Toast} from "./Toast"; \ No newline at end of file diff --git a/app/packages/components/src/components/index.ts b/app/packages/components/src/components/index.ts index a586e07609..6f46e99c49 100644 --- a/app/packages/components/src/components/index.ts +++ b/app/packages/components/src/components/index.ts @@ -37,5 +37,6 @@ export { default as TabOption } from "./TabOption"; export { default as TextField } from "./TextField"; export { default as ThemeProvider, useFont, useTheme } from "./ThemeProvider"; export { default as Tooltip } from "./Tooltip"; +export { Toast } from "./Toast"; export * from "./types"; From 9271cd3eb143a6789ce6cda584923347a031665b Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Fri, 20 Sep 2024 08:28:40 -0700 Subject: [PATCH 50/69] initial execution store --- fiftyone/factory/repo_factory.py | 19 +++ fiftyone/factory/repos/execution_store.py | 64 ++++++++ fiftyone/operators/store/__init__.py | 18 +++ fiftyone/operators/store/models.py | 64 ++++++++ fiftyone/operators/store/permissions.py | 47 ++++++ fiftyone/operators/store/store.py | 131 +++++++++++++++++ tests/unittests/execution_store_tests.py | 170 ++++++++++++++++++++++ 7 files changed, 513 insertions(+) create mode 100644 fiftyone/factory/repos/execution_store.py create mode 100644 fiftyone/operators/store/__init__.py create mode 100644 fiftyone/operators/store/models.py create mode 100644 fiftyone/operators/store/permissions.py create mode 100644 fiftyone/operators/store/store.py create mode 100644 tests/unittests/execution_store_tests.py diff --git a/fiftyone/factory/repo_factory.py b/fiftyone/factory/repo_factory.py index 2b031588da..314163544e 100644 --- a/fiftyone/factory/repo_factory.py +++ b/fiftyone/factory/repo_factory.py @@ -13,6 +13,10 @@ DelegatedOperationRepo, MongoDelegatedOperationRepo, ) +from fiftyone.factory.repos.execution_store import ( + ExecutionStoreRepo, + MongoExecutionStoreRepo, +) _db: Database = None @@ -44,3 +48,18 @@ def delegated_operation_repo() -> DelegatedOperationRepo: return RepositoryFactory.repos[ MongoDelegatedOperationRepo.COLLECTION_NAME ] + + @staticmethod + def execution_store_repo() -> ExecutionStoreRepo: + """Factory method for execution store repository.""" + if ( + MongoExecutionStoreRepo.COLLECTION_NAME + not in RepositoryFactory.repos + ): + RepositoryFactory.repos[ + MongoExecutionStoreRepo.COLLECTION_NAME + ] = MongoExecutionStoreRepo( + collection=_get_db()[MongoExecutionStoreRepo.COLLECTION_NAME] + ) + + return RepositoryFactory.repos[MongoExecutionStoreRepo.COLLECTION_NAME] diff --git a/fiftyone/factory/repos/execution_store.py b/fiftyone/factory/repos/execution_store.py new file mode 100644 index 0000000000..ab56ccc620 --- /dev/null +++ b/fiftyone/factory/repos/execution_store.py @@ -0,0 +1,64 @@ +""" +Execution store repository for handling storage and retrieval of stores. +""" + +from pymongo.collection import Collection +from fiftyone.operators.store.models import StoreDocument, KeyDocument + + +class ExecutionStoreRepo: + """Base class for execution store repositories.""" + + COLLECTION_NAME = "execution_store" + + def __init__(self, collection: Collection): + self._collection = collection + + def create_store(self, store_name, permissions=None): + """Creates a store in the execution store.""" + store_doc = StoreDocument( + store_name=store_name, permissions=permissions + ) + self._collection.insert_one(store_doc.dict()) + return store_doc + + def set_key(self, store_name, key, value, ttl=None): + """Sets a key in the specified store.""" + key_doc = KeyDocument(key=key, value=value, ttl=ttl) + self._collection.update_one( + {"store_name": store_name}, + {"$set": {f"keys.{key}": key_doc.dict()}}, + ) + return key_doc + + def get_key(self, store_name, key): + """Gets a key from the specified store.""" + store = self._collection.find_one({"store_name": store_name}) + if store and key in store.get("keys", {}): + return KeyDocument(**store["keys"][key]) + return None + + def update_ttl(self, store_name, key, ttl): + """Updates the TTL for a key.""" + self._collection.update_one( + {"store_name": store_name}, {"$set": {f"keys.{key}.ttl": ttl}} + ) + + def delete_key(self, store_name, key): + """Deletes a key from the store.""" + self._collection.update_one( + {"store_name": store_name}, {"$unset": {f"keys.{key}": ""}} + ) + + def delete_store(self, store_name): + """Deletes the entire store.""" + self._collection.delete_one({"store_name": store_name}) + + +class MongoExecutionStoreRepo(ExecutionStoreRepo): + """MongoDB implementation of execution store repository.""" + + COLLECTION_NAME = "execution_store" + + def __init__(self, collection: Collection): + super().__init__(collection) diff --git a/fiftyone/operators/store/__init__.py b/fiftyone/operators/store/__init__.py new file mode 100644 index 0000000000..34d7008577 --- /dev/null +++ b/fiftyone/operators/store/__init__.py @@ -0,0 +1,18 @@ +""" +FiftyOne execution store module. + +| Copyright 2017-2024, Voxel51, Inc. +| `voxel51.com `_ +| +""" + +from .store import ExecutionStoreService +from .models import StoreDocument, KeyDocument +from .permissions import StorePermissions + +__all__ = [ + "ExecutionStoreService", + "StoreDocument", + "KeyDocument", + "StorePermissions", +] diff --git a/fiftyone/operators/store/models.py b/fiftyone/operators/store/models.py new file mode 100644 index 0000000000..31a9ca0e87 --- /dev/null +++ b/fiftyone/operators/store/models.py @@ -0,0 +1,64 @@ +""" +Store and key models for the execution store. +""" + +from pydantic import BaseModel, Field +from typing import Optional, Dict, Any +import datetime + + +class KeyDocument(BaseModel): + """Model representing a key in the store.""" + + key: str + value: Any + ttl: Optional[int] = None # Time To Live in milliseconds + created_at: datetime.datetime = Field( + default_factory=datetime.datetime.utcnow + ) + updated_at: Optional[datetime.datetime] = None + + class Config: + schema_extra = { + "example": { + "key": "widgets_key", + "value": {"widget_1": "foo", "widget_2": "bar"}, + "ttl": 600000, # 10 minutes + } + } + + +class StoreDocument(BaseModel): + """Model representing a store in the execution store.""" + + store_name: str + keys: Dict[str, KeyDocument] = Field(default_factory=dict) + permissions: Optional[ + Dict[str, Any] + ] = None # Permissions can be a role/user/plugin map + created_at: datetime.datetime = Field( + default_factory=datetime.datetime.utcnow + ) + + class Config: + schema_extra = { + "example": { + "store_name": "widget_store", + "keys": { + "widgets_key": { + "key": "widgets_key", + "value": {"widget_1": "foo", "widget_2": "bar"}, + "ttl": 600000, + } + }, + "permissions": { + "roles": {"admin": ["read", "write"]}, + "users": {"user_1": ["read", "write"], "user_2": ["read"]}, + "groups": {"group_1": ["read", "write"]}, + "plugins": { + "plugin_1_uri": ["read"], + "plugin_2_uri": ["write"], + }, + }, + } + } diff --git a/fiftyone/operators/store/permissions.py b/fiftyone/operators/store/permissions.py new file mode 100644 index 0000000000..4601ec7bcb --- /dev/null +++ b/fiftyone/operators/store/permissions.py @@ -0,0 +1,47 @@ +""" +Permission models for execution store. +""" + +from pydantic import BaseModel +from typing import List, Optional, Dict + +from typing import Dict, List, Optional +from pydantic import BaseModel, Field + + +class StorePermissions(BaseModel): + """Model representing permissions for a store.""" + + roles: Optional[Dict[str, List[str]]] = Field(default_factory=dict) + users: Optional[Dict[str, List[str]]] = Field(default_factory=dict) + plugins: Optional[Dict[str, List[str]]] = Field(default_factory=dict) + + @staticmethod + def default(): + """Provides default permissions, allowing full access to the creator.""" + return StorePermissions( + roles={"admin": ["read", "write"]}, + users={}, + plugins={}, + ) + + def has_permission(self, entity, action): + """Checks if the given entity (role, user, or plugin) has the specified action permission. + + Args: + entity: The entity to check (can be a role, user, or plugin) + action: The action to check (e.g., 'read', 'write') + + Returns: + bool: True if the entity has permission, False otherwise + """ + return True # permission implementation TBD + + # class Config: + # schema_extra = { + # "example": { + # "roles": {"admin": ["read", "write"]}, + # "users": {"user_1": ["read", "write"], "user_2": ["read"]}, + # "plugins": {"plugin_1_uri": ["read"], "plugin_2_uri": ["write"]} + # } + # } diff --git a/fiftyone/operators/store/store.py b/fiftyone/operators/store/store.py new file mode 100644 index 0000000000..d8ae577975 --- /dev/null +++ b/fiftyone/operators/store/store.py @@ -0,0 +1,131 @@ +""" +FiftyOne execution store service. + +| Copyright 2017-2024, Voxel51, Inc. +| `voxel51.com `_ +| +""" + +import logging +import traceback +from fiftyone.factory.repo_factory import RepositoryFactory +from fiftyone.operators.store.models import StoreDocument, KeyDocument +from fiftyone.operators.store.permissions import StorePermissions + +logger = logging.getLogger(__name__) + + +class ExecutionStoreService(object): + """Service for managing execution store operations.""" + + def __init__(self, repo=None): + if repo is None: + repo = RepositoryFactory.execution_store_repo() + + self._repo = repo + + def create_store(self, store_name, permissions=None): + """Creates a new store with the specified name and permissions. + + Args: + store_name: the name of the store + permissions (None): an optional permissions dict + + Returns: + a :class:`fiftyone.store.models.StoreDocument` + """ + return self._repo.create_store( + store_name=store_name, + permissions=permissions or StorePermissions.default(), + ) + + def set_key(self, store_name, key, value, ttl=None): + """Sets the value of a key in the specified store. + + Args: + store_name: the name of the store + key: the key to set + value: the value to set + ttl (None): an optional TTL in milliseconds + + Returns: + a :class:`fiftyone.store.models.KeyDocument` + """ + return self._repo.set_key( + store_name=store_name, key=key, value=value, ttl=ttl + ) + + def get_key(self, store_name, key): + """Retrieves the value of a key from the specified store. + + Args: + store_name: the name of the store + key: the key to retrieve + + Returns: + a :class:`fiftyone.store.models.KeyDocument` + """ + return self._repo.get_key(store_name=store_name, key=key) + + def delete_key(self, store_name, key): + """Deletes the specified key from the store. + + Args: + store_name: the name of the store + key: the key to delete + + Returns: + a :class:`fiftyone.store.models.KeyDocument` + """ + return self._repo.delete_key(store_name=store_name, key=key) + + def update_ttl(self, store_name, key, new_ttl): + """Updates the TTL of the specified key in the store. + + Args: + store_name: the name of the store + key: the key to update the TTL for + new_ttl: the new TTL in milliseconds + + Returns: + a :class:`fiftyone.store.models.KeyDocument` + """ + return self._repo.update_ttl( + store_name=store_name, key=key, ttl=new_ttl + ) + + def set_permissions(self, store_name, permissions): + """Sets the permissions for the specified store. + + Args: + store_name: the name of the store + permissions: a permissions object + + Returns: + a :class:`fiftyone.store.models.StoreDocument` + """ + return self._repo.update_permissions( + store_name=store_name, permissions=permissions + ) + + def list_stores(self, search=None, **kwargs): + """Lists all stores matching the given criteria. + + Args: + search (None): optional search term dict + + Returns: + a list of :class:`fiftyone.store.models.StoreDocument` + """ + return self._repo.list_stores(search=search, **kwargs) + + def delete_store(self, store_name): + """Deletes the specified store. + + Args: + store_name: the name of the store + + Returns: + a :class:`fiftyone.store.models.StoreDocument` + """ + return self._repo.delete_store(store_name=store_name) diff --git a/tests/unittests/execution_store_tests.py b/tests/unittests/execution_store_tests.py new file mode 100644 index 0000000000..c324a1e497 --- /dev/null +++ b/tests/unittests/execution_store_tests.py @@ -0,0 +1,170 @@ +""" +FiftyOne execution store related unit tests. + +| Copyright 2017-2024, Voxel51, Inc. +| `voxel51.com `_ +| +""" + +import time +import unittest +from unittest import mock +from unittest.mock import patch + +import fiftyone +from bson import ObjectId + +from fiftyone.operators.store import ExecutionStoreService +from fiftyone.operators.store.models import StoreDocument, KeyDocument +from fiftyone.operators.store.permissions import StorePermissions + + +class MockStoreRepo: + def __init__(self): + self.stores = {} + + def create_store(self, store_name, permissions=None): + if isinstance(permissions, StorePermissions): + permissions = permissions.dict() + + store = StoreDocument(store_name=store_name, permissions=permissions) + self.stores[store_name] = store + return store + + def set_key(self, store_name, key, value, ttl=None): + store = self.stores.get(store_name) + if store: + key_doc = KeyDocument(key=key, value=value, ttl=ttl) + store.keys[key] = key_doc + return key_doc + return None + + def get_key(self, store_name, key): + store = self.stores.get(store_name) + if store and key in store.keys: + return store.keys[key] + return None + + def delete_key(self, store_name, key): + store = self.stores.get(store_name) + if store and key in store.keys: + del store.keys[key] + + def delete_store(self, store_name): + if store_name in self.stores: + del self.stores[store_name] + + def update_ttl(self, store_name, key, ttl): + store = self.stores.get(store_name) + if store and key in store.keys: + store.keys[key].ttl = ttl + return store.keys[key] + return None + + +class ExecutionStoreServiceTests(unittest.TestCase): + def setUp(self): + # Mock repository and service for testing + self.repo = MockStoreRepo() + self.svc = ExecutionStoreService(repo=self.repo) + + def test_create_store(self): + store_name = "test_store" + permissions = StorePermissions.default() + + store = self.svc.create_store( + store_name=store_name, permissions=permissions + ) + + self.assertIsNotNone(store) + self.assertEqual(store.store_name, store_name) + # Now compare the dictionary instead of the attribute + self.assertEqual(store.permissions["roles"], permissions.roles) + + def test_set_and_get_key(self): + store_name = "test_store" + self.svc.create_store(store_name=store_name) + + key = "test_key" + value = {"foo": "bar"} + ttl = 10000 + + key_doc = self.svc.set_key( + store_name=store_name, key=key, value=value, ttl=ttl + ) + self.assertIsNotNone(key_doc) + self.assertEqual(key_doc.key, key) + self.assertEqual(key_doc.value, value) + self.assertEqual(key_doc.ttl, ttl) + + # Get the key and validate the value + fetched_key_doc = self.svc.get_key(store_name=store_name, key=key) + self.assertIsNotNone(fetched_key_doc) + self.assertEqual(fetched_key_doc.key, key) + self.assertEqual(fetched_key_doc.value, value) + + def test_delete_key(self): + store_name = "test_store" + self.svc.create_store(store_name=store_name) + + key = "test_key" + value = {"foo": "bar"} + self.svc.set_key(store_name=store_name, key=key, value=value) + + # Ensure the key exists + key_doc = self.svc.get_key(store_name=store_name, key=key) + self.assertIsNotNone(key_doc) + + # Delete the key + self.svc.delete_key(store_name=store_name, key=key) + + # Ensure the key is deleted + key_doc = self.svc.get_key(store_name=store_name, key=key) + self.assertIsNone(key_doc) + + def test_update_ttl(self): + store_name = "test_store" + self.svc.create_store(store_name=store_name) + + key = "test_key" + value = {"foo": "bar"} + self.svc.set_key(store_name=store_name, key=key, value=value) + + # Update TTL + new_ttl = 5000 + self.svc.update_ttl(store_name=store_name, key=key, new_ttl=new_ttl) + + # Fetch key and check updated TTL + key_doc = self.svc.get_key(store_name=store_name, key=key) + self.assertIsNotNone(key_doc) + self.assertEqual(key_doc.ttl, new_ttl) + + def test_delete_store(self): + store_name = "test_store" + self.svc.create_store(store_name=store_name) + + # Ensure the store exists + store = self.repo.stores.get(store_name) + self.assertIsNotNone(store) + + # Delete the store + self.svc.delete_store(store_name=store_name) + + # Ensure the store is deleted + store = self.repo.stores.get(store_name) + self.assertIsNone(store) + + def test_store_permissions(self): + store_name = "test_store" + permissions = StorePermissions.default() + self.svc.create_store(store_name=store_name, permissions=permissions) + + # Check default permissions + store = self.repo.stores.get(store_name) + self.assertIsNotNone(store) + # Now compare the dictionary instead of the attribute + self.assertEqual(store.permissions["roles"], permissions.roles) + + +if __name__ == "__main__": + unittest.main() From 0eaf6e3c83be0710333adac403f27e024244209e Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Fri, 20 Sep 2024 09:54:59 -0700 Subject: [PATCH 51/69] initial int tests for execution store --- fiftyone/factory/repos/execution_store.py | 51 +++++---- fiftyone/operators/executor.py | 14 +++ fiftyone/operators/store/__init__.py | 4 +- fiftyone/operators/store/models.py | 25 ++-- fiftyone/operators/store/service.py | 130 +++++++++++++++++++++ fiftyone/operators/store/store.py | 133 ++++++++-------------- tests/unittests/execution_store_tests.py | 129 ++++++++++++++++++++- 7 files changed, 361 insertions(+), 125 deletions(-) create mode 100644 fiftyone/operators/store/service.py diff --git a/fiftyone/factory/repos/execution_store.py b/fiftyone/factory/repos/execution_store.py index ab56ccc620..abe86ee956 100644 --- a/fiftyone/factory/repos/execution_store.py +++ b/fiftyone/factory/repos/execution_store.py @@ -6,6 +6,13 @@ from fiftyone.operators.store.models import StoreDocument, KeyDocument +def _where(store_name, key=None): + query = {"store_name": store_name} + if key is not None: + query["key"] = key + return query + + class ExecutionStoreRepo: """Base class for execution store repositories.""" @@ -14,7 +21,7 @@ class ExecutionStoreRepo: def __init__(self, collection: Collection): self._collection = collection - def create_store(self, store_name, permissions=None): + def create_store(self, store_name, permissions=None) -> StoreDocument: """Creates a store in the execution store.""" store_doc = StoreDocument( store_name=store_name, permissions=permissions @@ -22,37 +29,39 @@ def create_store(self, store_name, permissions=None): self._collection.insert_one(store_doc.dict()) return store_doc - def set_key(self, store_name, key, value, ttl=None): - """Sets a key in the specified store.""" - key_doc = KeyDocument(key=key, value=value, ttl=ttl) + def set_key(self, store_name, key, value, ttl=None) -> KeyDocument: + """Sets or updates a key in the specified store.""" + key_doc = KeyDocument( + store_name=store_name, key=key, value=value, ttl=ttl + ) + # Update or insert the key self._collection.update_one( - {"store_name": store_name}, - {"$set": {f"keys.{key}": key_doc.dict()}}, + _where(store_name, key), {"$set": key_doc.dict()}, upsert=True ) return key_doc - def get_key(self, store_name, key): + def get_key(self, store_name, key) -> KeyDocument: """Gets a key from the specified store.""" - store = self._collection.find_one({"store_name": store_name}) - if store and key in store.get("keys", {}): - return KeyDocument(**store["keys"][key]) - return None + raw_key_doc = self._collection.find_one(_where(store_name, key)) + key_doc = KeyDocument(**raw_key_doc) if raw_key_doc else None + return key_doc - def update_ttl(self, store_name, key, ttl): + def update_ttl(self, store_name, key, ttl) -> bool: """Updates the TTL for a key.""" - self._collection.update_one( - {"store_name": store_name}, {"$set": {f"keys.{key}.ttl": ttl}} + result = self._collection.update_one( + _where(store_name, key), {"$set": {"ttl": ttl}} ) + return result.modified_count > 0 - def delete_key(self, store_name, key): - """Deletes a key from the store.""" - self._collection.update_one( - {"store_name": store_name}, {"$unset": {f"keys.{key}": ""}} - ) + def delete_key(self, store_name, key) -> bool: + """Deletes the document that matches the store name and key.""" + result = self._collection.delete_one(_where(store_name, key)) + return result.deleted_count > 0 - def delete_store(self, store_name): + def delete_store(self, store_name) -> int: """Deletes the entire store.""" - self._collection.delete_one({"store_name": store_name}) + result = self._collection.delete_many(_where(store_name)) + return result.deleted_count class MongoExecutionStoreRepo(ExecutionStoreRepo): diff --git a/fiftyone/operators/executor.py b/fiftyone/operators/executor.py index 3af9f377a9..15eeef3bb2 100644 --- a/fiftyone/operators/executor.py +++ b/fiftyone/operators/executor.py @@ -859,6 +859,20 @@ def set_progress(self, progress=None, label=None): else: self.log(f"Progress: {progress} - {label}") + # TODO resolve circular import so this can have a type + def create_store(self, store_name): + """Creates a new store with the specified name. + + Args: + store_name: the name of the store + + Returns: + a :class:`fiftyone.operators.store.ExecutionStore` + """ + from fiftyone.operators.store import ExecutionStore + + return ExecutionStore.create(store_name) + def serialize(self): """Serializes the execution context. diff --git a/fiftyone/operators/store/__init__.py b/fiftyone/operators/store/__init__.py index 34d7008577..0c33732c4f 100644 --- a/fiftyone/operators/store/__init__.py +++ b/fiftyone/operators/store/__init__.py @@ -6,7 +6,8 @@ | """ -from .store import ExecutionStoreService +from .service import ExecutionStoreService +from .store import ExecutionStore from .models import StoreDocument, KeyDocument from .permissions import StorePermissions @@ -15,4 +16,5 @@ "StoreDocument", "KeyDocument", "StorePermissions", + "ExecutionStore", ] diff --git a/fiftyone/operators/store/models.py b/fiftyone/operators/store/models.py index 31a9ca0e87..1ba3005abf 100644 --- a/fiftyone/operators/store/models.py +++ b/fiftyone/operators/store/models.py @@ -10,6 +10,7 @@ class KeyDocument(BaseModel): """Model representing a key in the store.""" + store_name: str key: str value: Any ttl: Optional[int] = None # Time To Live in milliseconds @@ -17,10 +18,14 @@ class KeyDocument(BaseModel): default_factory=datetime.datetime.utcnow ) updated_at: Optional[datetime.datetime] = None + permissions: Optional[ + Dict[str, Any] + ] = None # Permissions can be a role/user/plugin map class Config: schema_extra = { "example": { + "store_name": "widget_store", "key": "widgets_key", "value": {"widget_1": "foo", "widget_2": "bar"}, "ttl": 600000, # 10 minutes @@ -28,29 +33,17 @@ class Config: } -class StoreDocument(BaseModel): +class StoreDocument(KeyDocument): """Model representing a store in the execution store.""" - store_name: str - keys: Dict[str, KeyDocument] = Field(default_factory=dict) - permissions: Optional[ - Dict[str, Any] - ] = None # Permissions can be a role/user/plugin map - created_at: datetime.datetime = Field( - default_factory=datetime.datetime.utcnow - ) + key: str = "__store__" + value: Optional[Dict[str, Any]] = None class Config: schema_extra = { "example": { + "key": "__store__", "store_name": "widget_store", - "keys": { - "widgets_key": { - "key": "widgets_key", - "value": {"widget_1": "foo", "widget_2": "bar"}, - "ttl": 600000, - } - }, "permissions": { "roles": {"admin": ["read", "write"]}, "users": {"user_1": ["read", "write"], "user_2": ["read"]}, diff --git a/fiftyone/operators/store/service.py b/fiftyone/operators/store/service.py new file mode 100644 index 0000000000..9da28053a2 --- /dev/null +++ b/fiftyone/operators/store/service.py @@ -0,0 +1,130 @@ +""" +FiftyOne execution store service. + +| Copyright 2017-2024, Voxel51, Inc. +| `voxel51.com `_ +| +""" + +import logging +import traceback +from fiftyone.factory.repo_factory import RepositoryFactory +from fiftyone.operators.store.permissions import StorePermissions + +logger = logging.getLogger(__name__) + + +class ExecutionStoreService(object): + """Service for managing execution store operations.""" + + def __init__(self, repo=None): + if repo is None: + repo = RepositoryFactory.execution_store_repo() + + self._repo = repo + + def create_store(self, store_name, permissions=None): + """Creates a new store with the specified name and permissions. + + Args: + store_name: the name of the store + permissions (None): an optional permissions dict + + Returns: + a :class:`fiftyone.store.models.StoreDocument` + """ + return self._repo.create_store( + store_name=store_name, + permissions=permissions or StorePermissions.default(), + ) + + def set_key(self, store_name, key, value, ttl=None): + """Sets the value of a key in the specified store. + + Args: + store_name: the name of the store + key: the key to set + value: the value to set + ttl (None): an optional TTL in milliseconds + + Returns: + a :class:`fiftyone.store.models.KeyDocument` + """ + return self._repo.set_key( + store_name=store_name, key=key, value=value, ttl=ttl + ) + + def get_key(self, store_name, key): + """Retrieves the value of a key from the specified store. + + Args: + store_name: the name of the store + key: the key to retrieve + + Returns: + a :class:`fiftyone.store.models.KeyDocument` + """ + return self._repo.get_key(store_name=store_name, key=key) + + def delete_key(self, store_name, key): + """Deletes the specified key from the store. + + Args: + store_name: the name of the store + key: the key to delete + + Returns: + a :class:`fiftyone.store.models.KeyDocument` + """ + return self._repo.delete_key(store_name=store_name, key=key) + + def update_ttl(self, store_name, key, new_ttl): + """Updates the TTL of the specified key in the store. + + Args: + store_name: the name of the store + key: the key to update the TTL for + new_ttl: the new TTL in milliseconds + + Returns: + a :class:`fiftyone.store.models.KeyDocument` + """ + return self._repo.update_ttl( + store_name=store_name, key=key, ttl=new_ttl + ) + + def set_permissions(self, store_name, permissions): + """Sets the permissions for the specified store. + + Args: + store_name: the name of the store + permissions: a permissions object + + Returns: + a :class:`fiftyone.store.models.StoreDocument` + """ + return self._repo.update_permissions( + store_name=store_name, permissions=permissions + ) + + def list_stores(self, search=None, **kwargs): + """Lists all stores matching the given criteria. + + Args: + search (None): optional search term dict + + Returns: + a list of :class:`fiftyone.store.models.StoreDocument` + """ + return self._repo.list_stores(search=search, **kwargs) + + def delete_store(self, store_name): + """Deletes the specified store. + + Args: + store_name: the name of the store + + Returns: + a :class:`fiftyone.store.models.StoreDocument` + """ + return self._repo.delete_store(store_name=store_name) diff --git a/fiftyone/operators/store/store.py b/fiftyone/operators/store/store.py index d8ae577975..bb5077fe8c 100644 --- a/fiftyone/operators/store/store.py +++ b/fiftyone/operators/store/store.py @@ -1,5 +1,5 @@ """ -FiftyOne execution store service. +FiftyOne execution store. | Copyright 2017-2024, Voxel51, Inc. | `voxel51.com `_ @@ -7,125 +7,90 @@ """ import logging -import traceback from fiftyone.factory.repo_factory import RepositoryFactory -from fiftyone.operators.store.models import StoreDocument, KeyDocument -from fiftyone.operators.store.permissions import StorePermissions +from fiftyone.operators.store.service import ExecutionStoreService +from typing import Any, Optional logger = logging.getLogger(__name__) -class ExecutionStoreService(object): - """Service for managing execution store operations.""" +class ExecutionStore: + @staticmethod + def create(store_name: str) -> "ExecutionStore": + return ExecutionStore(store_name, ExecutionStoreService()) - def __init__(self, repo=None): - if repo is None: - repo = RepositoryFactory.execution_store_repo() - - self._repo = repo - - def create_store(self, store_name, permissions=None): - """Creates a new store with the specified name and permissions. - - Args: - store_name: the name of the store - permissions (None): an optional permissions dict - - Returns: - a :class:`fiftyone.store.models.StoreDocument` + def __init__(self, store_name: str, store_service: ExecutionStoreService): """ - return self._repo.create_store( - store_name=store_name, - permissions=permissions or StorePermissions.default(), - ) - - def set_key(self, store_name, key, value, ttl=None): - """Sets the value of a key in the specified store. - Args: - store_name: the name of the store - key: the key to set - value: the value to set - ttl (None): an optional TTL in milliseconds - - Returns: - a :class:`fiftyone.store.models.KeyDocument` + store_name (str): The name of the store. + store_service (ExecutionStoreService): The store service instance. """ - return self._repo.set_key( - store_name=store_name, key=key, value=value, ttl=ttl - ) + self.store_name: str = store_name + self._store_service: ExecutionStoreService = store_service - def get_key(self, store_name, key): - """Retrieves the value of a key from the specified store. + def get(self, key: str) -> Optional[Any]: + """Retrieves a value from the store by its key. Args: - store_name: the name of the store - key: the key to retrieve + key (str): The key to retrieve the value for. Returns: - a :class:`fiftyone.store.models.KeyDocument` + Optional[Any]: The value stored under the given key, or None if not found. """ - return self._repo.get_key(store_name=store_name, key=key) + key_doc = self._store_service.get_key(self.store_name, key) + if key_doc is None: + return None + return key_doc.value - def delete_key(self, store_name, key): - """Deletes the specified key from the store. + def set(self, key: str, value: Any, ttl: Optional[int] = None) -> None: + """Sets a value in the store with an optional TTL. Args: - store_name: the name of the store - key: the key to delete - - Returns: - a :class:`fiftyone.store.models.KeyDocument` + key (str): The key to store the value under. + value (Any): The value to store. + ttl (Optional[int], optional): The time-to-live in milliseconds. Defaults to None. """ - return self._repo.delete_key(store_name=store_name, key=key) + self._store_service.set_key(self.store_name, key, value, ttl) - def update_ttl(self, store_name, key, new_ttl): - """Updates the TTL of the specified key in the store. + def delete(self, key: str) -> None: + """Deletes a key from the store. Args: - store_name: the name of the store - key: the key to update the TTL for - new_ttl: the new TTL in milliseconds - - Returns: - a :class:`fiftyone.store.models.KeyDocument` + key (str): The key to delete. """ - return self._repo.update_ttl( - store_name=store_name, key=key, ttl=new_ttl - ) + self._store_service.delete_key(self.store_name, key) - def set_permissions(self, store_name, permissions): - """Sets the permissions for the specified store. + def has(self, key: str) -> bool: + """Checks if the store has a specific key. Args: - store_name: the name of the store - permissions: a permissions object + key (str): The key to check. Returns: - a :class:`fiftyone.store.models.StoreDocument` + bool: True if the key exists, False otherwise. """ - return self._repo.update_permissions( - store_name=store_name, permissions=permissions - ) + return self._store_service.has_key(self.store_name, key) - def list_stores(self, search=None, **kwargs): - """Lists all stores matching the given criteria. + def clear(self) -> None: + """Clears all the data in the store.""" + self._store_service.clear_store(self.store_name) - Args: - search (None): optional search term dict + def update_ttl(self, key: str, new_ttl: int) -> None: + """Updates the TTL for a specific key. - Returns: - a list of :class:`fiftyone.store.models.StoreDocument` + Args: + key (str): The key to update the TTL for. + new_ttl (int): The new TTL in milliseconds. """ - return self._repo.list_stores(search=search, **kwargs) + self._store_service.update_ttl(self.store_name, key, new_ttl) - def delete_store(self, store_name): - """Deletes the specified store. + def get_ttl(self, key: str) -> Optional[int]: + """Retrieves the TTL for a specific key. Args: - store_name: the name of the store + key (str): The key to get the TTL for. Returns: - a :class:`fiftyone.store.models.StoreDocument` + Optional[int]: The TTL in milliseconds, or None if the key does not have a TTL. """ - return self._repo.delete_store(store_name=store_name) + return self._store_service.get_ttl(self.store_name, key) diff --git a/tests/unittests/execution_store_tests.py b/tests/unittests/execution_store_tests.py index c324a1e497..1ad21aa972 100644 --- a/tests/unittests/execution_store_tests.py +++ b/tests/unittests/execution_store_tests.py @@ -9,14 +9,37 @@ import time import unittest from unittest import mock -from unittest.mock import patch +from unittest.mock import patch, MagicMock import fiftyone from bson import ObjectId +import fiftyone.operators as foo from fiftyone.operators.store import ExecutionStoreService from fiftyone.operators.store.models import StoreDocument, KeyDocument from fiftyone.operators.store.permissions import StorePermissions +from fiftyone.factory.repo_factory import ExecutionStoreRepo +from fiftyone.operators.operator import Operator +from fiftyone.operators.store import ExecutionStoreService + + +class MockOperator: + """Mock operator that interacts with ExecutionStore.""" + + def __init__(self, ctx: foo.ExecutionContext): + self.ctx = ctx + + def execute(self): + # Example logic of interacting with the store + store = self.ctx.create_store("widgets") + + # Set a value in the store + store.set("widget_1", {"name": "Widget One", "value": 100}, ttl=60000) + + # Get the value back from the store + result = store.get("widget_1") + + return result class MockStoreRepo: @@ -166,5 +189,105 @@ def test_store_permissions(self): self.assertEqual(store.permissions["roles"], permissions.roles) -if __name__ == "__main__": - unittest.main() +class TestOperatorWithExecutionStore(Operator): + def execute(self, ctx): + # Create a store and interact with it + store = ctx.create_store("test_store") + + # Perform some store operations + store.set("my_key", {"foo": "bar"}) + value = store.get("my_key") + store.delete("my_key") + + return value + + +class ExecutionStoreIntegrationTests(unittest.TestCase): + def setUp(self): + # Create a MagicMock for the MongoDB collection + self.mock_collection = MagicMock() + + # Instantiate the store repo and replace the _collection attribute + self.store_repo = ExecutionStoreRepo(self.mock_collection) + + # Create the store service with the mocked repo + self.store_service = ExecutionStoreService(self.store_repo) + + # Create an execution context + from fiftyone.operators import ExecutionContext + + self.ctx = ExecutionContext() + self.ctx.store_service = ( + self.store_service + ) # Inject the store service into the context + + # Create an instance of the operator + self.operator = TestOperatorWithExecutionStore() + + def test_operator_execute_with_store(self): + # Mock MongoDB collection methods + self.mock_collection.update_one.return_value = None + self.mock_collection.find_one.return_value = { + "key": "my_key", + "value": {"foo": "bar"}, + } + self.mock_collection.delete_one.return_value = None + + # Call the operator's execute method + result = self.operator.execute(self.ctx) + + # Verify that the store interactions were made correctly + self.mock_collection.update_one.assert_called_once() # Checking that set operation inserts data + self.mock_collection.find_one.assert_called_once_with( + {"store_name": "test_store", "key": "my_key"} + ) # Checking that get operation retrieves data + self.mock_collection.delete_one.assert_called_once_with( + {"store_name": "test_store", "key": "my_key"} + ) # Checking that delete operation removes data + + # Verify the correct value was returned from the store + self.assertEqual(result, {"key": "my_key", "value": {"foo": "bar"}}) + + def test_operator_execute_set_key(self): + # Mock the insert_one call for the set operation + self.mock_collection.insert_one.return_value = None + + # Call the operator's execute method + self.operator.execute(self.ctx) + + # Check that insert_one (set) was called with the correct arguments + self.mock_collection.insert_one.assert_called_once_with( + { + "store_name": "test_store", + "key": "my_key", + "value": {"foo": "bar"}, + } + ) + + def test_operator_execute_get_key(self): + # Mock the find_one call for the get operation + self.mock_collection.find_one.return_value = { + "key": "my_key", + "value": {"foo": "bar"}, + } + + # Call the operator's execute method + result = self.operator.execute(self.ctx) + + # Check that find_one (get) was called correctly and returned the expected value + self.mock_collection.find_one.assert_called_once_with( + {"store_name": "test_store", "key": "my_key"} + ) + self.assertEqual(result, {"key": "my_key", "value": {"foo": "bar"}}) + + def test_operator_execute_delete_key(self): + # Mock the delete_one call for the delete operation + self.mock_collection.delete_one.return_value = None + + # Call the operator's execute method + self.operator.execute(self.ctx) + + # Check that delete_one was called with the correct arguments + self.mock_collection.delete_one.assert_called_once_with( + {"store_name": "test_store", "key": "my_key"} + ) From dd2f93db17edc6c50ee786ebfa54c32a5517adf2 Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Sun, 22 Sep 2024 13:48:25 -0700 Subject: [PATCH 52/69] fixes for execution store --- fiftyone/factory/repo_factory.py | 1 + fiftyone/factory/repos/execution_store.py | 24 +++++++++++++++++++++-- fiftyone/operators/store/models.py | 18 ++++++++--------- fiftyone/operators/store/service.py | 8 ++++++++ fiftyone/operators/store/store.py | 8 ++++++++ 5 files changed, 47 insertions(+), 12 deletions(-) diff --git a/fiftyone/factory/repo_factory.py b/fiftyone/factory/repo_factory.py index 314163544e..05a875fce1 100644 --- a/fiftyone/factory/repo_factory.py +++ b/fiftyone/factory/repo_factory.py @@ -52,6 +52,7 @@ def delegated_operation_repo() -> DelegatedOperationRepo: @staticmethod def execution_store_repo() -> ExecutionStoreRepo: """Factory method for execution store repository.""" + print("Factory method for execution store repository.") if ( MongoExecutionStoreRepo.COLLECTION_NAME not in RepositoryFactory.repos diff --git a/fiftyone/factory/repos/execution_store.py b/fiftyone/factory/repos/execution_store.py index abe86ee956..0b2e5aad9c 100644 --- a/fiftyone/factory/repos/execution_store.py +++ b/fiftyone/factory/repos/execution_store.py @@ -31,8 +31,12 @@ def create_store(self, store_name, permissions=None) -> StoreDocument: def set_key(self, store_name, key, value, ttl=None) -> KeyDocument: """Sets or updates a key in the specified store.""" + expiration = KeyDocument.get_expiration(ttl) key_doc = KeyDocument( - store_name=store_name, key=key, value=value, ttl=ttl + store_name=store_name, + key=key, + value=value, + expires_at=expiration if ttl else None, ) # Update or insert the key self._collection.update_one( @@ -46,10 +50,17 @@ def get_key(self, store_name, key) -> KeyDocument: key_doc = KeyDocument(**raw_key_doc) if raw_key_doc else None return key_doc + def list_keys(self, store_name) -> list[str]: + """Lists all keys in the specified store.""" + keys = self._collection.find(_where(store_name)) + # TODO: redact non-key fields + return [key["key"] for key in keys] + def update_ttl(self, store_name, key, ttl) -> bool: """Updates the TTL for a key.""" + expiration = KeyDocument.get_expiration(ttl) result = self._collection.update_one( - _where(store_name, key), {"$set": {"ttl": ttl}} + _where(store_name, key), {"$set": {"expires_at": expiration}} ) return result.modified_count > 0 @@ -71,3 +82,12 @@ class MongoExecutionStoreRepo(ExecutionStoreRepo): def __init__(self, collection: Collection): super().__init__(collection) + self._create_indexes() + + def _create_indexes(self): + indices = self._collection.list_indexes() + expires_at_name = "expires_at" + if expires_at_name not in indices: + self._collection.create_index( + expires_at_name, name=expires_at_name, expireAfterSeconds=0 + ) diff --git a/fiftyone/operators/store/models.py b/fiftyone/operators/store/models.py index 1ba3005abf..e2502a98ba 100644 --- a/fiftyone/operators/store/models.py +++ b/fiftyone/operators/store/models.py @@ -13,24 +13,22 @@ class KeyDocument(BaseModel): store_name: str key: str value: Any - ttl: Optional[int] = None # Time To Live in milliseconds created_at: datetime.datetime = Field( default_factory=datetime.datetime.utcnow ) updated_at: Optional[datetime.datetime] = None + expires_at: Optional[datetime.datetime] = None permissions: Optional[ Dict[str, Any] ] = None # Permissions can be a role/user/plugin map - class Config: - schema_extra = { - "example": { - "store_name": "widget_store", - "key": "widgets_key", - "value": {"widget_1": "foo", "widget_2": "bar"}, - "ttl": 600000, # 10 minutes - } - } + @staticmethod + def get_expiration(ttl: Optional[int]) -> Optional[datetime.datetime]: + """Gets the expiration date for a key with the given TTL.""" + if ttl is None: + return None + + return datetime.datetime.now() + datetime.timedelta(milliseconds=ttl) class StoreDocument(KeyDocument): diff --git a/fiftyone/operators/store/service.py b/fiftyone/operators/store/service.py index 9da28053a2..0cebd197f7 100644 --- a/fiftyone/operators/store/service.py +++ b/fiftyone/operators/store/service.py @@ -128,3 +128,11 @@ def delete_store(self, store_name): a :class:`fiftyone.store.models.StoreDocument` """ return self._repo.delete_store(store_name=store_name) + + def list_keys(self, store_name): + """Lists all keys in the specified store. + + Args: + store_name: the name of the store + """ + return self._repo.list_keys(store_name) diff --git a/fiftyone/operators/store/store.py b/fiftyone/operators/store/store.py index bb5077fe8c..d0fc738f9f 100644 --- a/fiftyone/operators/store/store.py +++ b/fiftyone/operators/store/store.py @@ -94,3 +94,11 @@ def get_ttl(self, key: str) -> Optional[int]: Optional[int]: The TTL in milliseconds, or None if the key does not have a TTL. """ return self._store_service.get_ttl(self.store_name, key) + + def list_keys(self) -> list[str]: + """Lists all keys in the store. + + Returns: + list: A list of keys in the store. + """ + return self._store_service.list_keys(self.store_name) From d0a571214ac4fdff671fe83b426e992291f4c812 Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Sun, 22 Sep 2024 14:34:53 -0700 Subject: [PATCH 53/69] exec store cleanup --- fiftyone/factory/repo_factory.py | 1 - fiftyone/operators/store/__init__.py | 2 - fiftyone/operators/store/models.py | 17 --------- fiftyone/operators/store/permissions.py | 47 ------------------------ fiftyone/operators/store/service.py | 26 ++----------- tests/unittests/execution_store_tests.py | 24 ++---------- 6 files changed, 7 insertions(+), 110 deletions(-) delete mode 100644 fiftyone/operators/store/permissions.py diff --git a/fiftyone/factory/repo_factory.py b/fiftyone/factory/repo_factory.py index 05a875fce1..314163544e 100644 --- a/fiftyone/factory/repo_factory.py +++ b/fiftyone/factory/repo_factory.py @@ -52,7 +52,6 @@ def delegated_operation_repo() -> DelegatedOperationRepo: @staticmethod def execution_store_repo() -> ExecutionStoreRepo: """Factory method for execution store repository.""" - print("Factory method for execution store repository.") if ( MongoExecutionStoreRepo.COLLECTION_NAME not in RepositoryFactory.repos diff --git a/fiftyone/operators/store/__init__.py b/fiftyone/operators/store/__init__.py index 0c33732c4f..50f9f3b400 100644 --- a/fiftyone/operators/store/__init__.py +++ b/fiftyone/operators/store/__init__.py @@ -9,12 +9,10 @@ from .service import ExecutionStoreService from .store import ExecutionStore from .models import StoreDocument, KeyDocument -from .permissions import StorePermissions __all__ = [ "ExecutionStoreService", "StoreDocument", "KeyDocument", - "StorePermissions", "ExecutionStore", ] diff --git a/fiftyone/operators/store/models.py b/fiftyone/operators/store/models.py index e2502a98ba..dab61012d4 100644 --- a/fiftyone/operators/store/models.py +++ b/fiftyone/operators/store/models.py @@ -36,20 +36,3 @@ class StoreDocument(KeyDocument): key: str = "__store__" value: Optional[Dict[str, Any]] = None - - class Config: - schema_extra = { - "example": { - "key": "__store__", - "store_name": "widget_store", - "permissions": { - "roles": {"admin": ["read", "write"]}, - "users": {"user_1": ["read", "write"], "user_2": ["read"]}, - "groups": {"group_1": ["read", "write"]}, - "plugins": { - "plugin_1_uri": ["read"], - "plugin_2_uri": ["write"], - }, - }, - } - } diff --git a/fiftyone/operators/store/permissions.py b/fiftyone/operators/store/permissions.py deleted file mode 100644 index 4601ec7bcb..0000000000 --- a/fiftyone/operators/store/permissions.py +++ /dev/null @@ -1,47 +0,0 @@ -""" -Permission models for execution store. -""" - -from pydantic import BaseModel -from typing import List, Optional, Dict - -from typing import Dict, List, Optional -from pydantic import BaseModel, Field - - -class StorePermissions(BaseModel): - """Model representing permissions for a store.""" - - roles: Optional[Dict[str, List[str]]] = Field(default_factory=dict) - users: Optional[Dict[str, List[str]]] = Field(default_factory=dict) - plugins: Optional[Dict[str, List[str]]] = Field(default_factory=dict) - - @staticmethod - def default(): - """Provides default permissions, allowing full access to the creator.""" - return StorePermissions( - roles={"admin": ["read", "write"]}, - users={}, - plugins={}, - ) - - def has_permission(self, entity, action): - """Checks if the given entity (role, user, or plugin) has the specified action permission. - - Args: - entity: The entity to check (can be a role, user, or plugin) - action: The action to check (e.g., 'read', 'write') - - Returns: - bool: True if the entity has permission, False otherwise - """ - return True # permission implementation TBD - - # class Config: - # schema_extra = { - # "example": { - # "roles": {"admin": ["read", "write"]}, - # "users": {"user_1": ["read", "write"], "user_2": ["read"]}, - # "plugins": {"plugin_1_uri": ["read"], "plugin_2_uri": ["write"]} - # } - # } diff --git a/fiftyone/operators/store/service.py b/fiftyone/operators/store/service.py index 0cebd197f7..47dcf1ada3 100644 --- a/fiftyone/operators/store/service.py +++ b/fiftyone/operators/store/service.py @@ -7,9 +7,7 @@ """ import logging -import traceback from fiftyone.factory.repo_factory import RepositoryFactory -from fiftyone.operators.store.permissions import StorePermissions logger = logging.getLogger(__name__) @@ -23,20 +21,16 @@ def __init__(self, repo=None): self._repo = repo - def create_store(self, store_name, permissions=None): - """Creates a new store with the specified name and permissions. + def create_store(self, store_name): + """Creates a new store with the specified name. Args: store_name: the name of the store - permissions (None): an optional permissions dict Returns: a :class:`fiftyone.store.models.StoreDocument` """ - return self._repo.create_store( - store_name=store_name, - permissions=permissions or StorePermissions.default(), - ) + return self._repo.create_store(store_name=store_name) def set_key(self, store_name, key, value, ttl=None): """Sets the value of a key in the specified store. @@ -93,20 +87,6 @@ def update_ttl(self, store_name, key, new_ttl): store_name=store_name, key=key, ttl=new_ttl ) - def set_permissions(self, store_name, permissions): - """Sets the permissions for the specified store. - - Args: - store_name: the name of the store - permissions: a permissions object - - Returns: - a :class:`fiftyone.store.models.StoreDocument` - """ - return self._repo.update_permissions( - store_name=store_name, permissions=permissions - ) - def list_stores(self, search=None, **kwargs): """Lists all stores matching the given criteria. diff --git a/tests/unittests/execution_store_tests.py b/tests/unittests/execution_store_tests.py index 1ad21aa972..12eb3a4c69 100644 --- a/tests/unittests/execution_store_tests.py +++ b/tests/unittests/execution_store_tests.py @@ -11,13 +11,9 @@ from unittest import mock from unittest.mock import patch, MagicMock -import fiftyone -from bson import ObjectId - import fiftyone.operators as foo from fiftyone.operators.store import ExecutionStoreService from fiftyone.operators.store.models import StoreDocument, KeyDocument -from fiftyone.operators.store.permissions import StorePermissions from fiftyone.factory.repo_factory import ExecutionStoreRepo from fiftyone.operators.operator import Operator from fiftyone.operators.store import ExecutionStoreService @@ -46,11 +42,8 @@ class MockStoreRepo: def __init__(self): self.stores = {} - def create_store(self, store_name, permissions=None): - if isinstance(permissions, StorePermissions): - permissions = permissions.dict() - - store = StoreDocument(store_name=store_name, permissions=permissions) + def create_store(self, store_name): + store = StoreDocument(store_name=store_name) self.stores[store_name] = store return store @@ -93,16 +86,10 @@ def setUp(self): def test_create_store(self): store_name = "test_store" - permissions = StorePermissions.default() - - store = self.svc.create_store( - store_name=store_name, permissions=permissions - ) + store = self.svc.create_store(store_name=store_name) self.assertIsNotNone(store) self.assertEqual(store.store_name, store_name) - # Now compare the dictionary instead of the attribute - self.assertEqual(store.permissions["roles"], permissions.roles) def test_set_and_get_key(self): store_name = "test_store" @@ -179,14 +166,11 @@ def test_delete_store(self): def test_store_permissions(self): store_name = "test_store" - permissions = StorePermissions.default() - self.svc.create_store(store_name=store_name, permissions=permissions) + self.svc.create_store(store_name=store_name) # Check default permissions store = self.repo.stores.get(store_name) self.assertIsNotNone(store) - # Now compare the dictionary instead of the attribute - self.assertEqual(store.permissions["roles"], permissions.roles) class TestOperatorWithExecutionStore(Operator): From 6c1d3ea529576cb3c822729d83d2298392c6048b Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Sun, 22 Sep 2024 16:22:02 -0700 Subject: [PATCH 54/69] better exec store tests --- fiftyone/operators/store/models.py | 7 +- fiftyone/operators/store/service.py | 4 +- fiftyone/operators/store/store.py | 7 +- tests/unittests/execution_store_tests.py | 400 ++++++++++------------- 4 files changed, 187 insertions(+), 231 deletions(-) diff --git a/fiftyone/operators/store/models.py b/fiftyone/operators/store/models.py index dab61012d4..e05767fe22 100644 --- a/fiftyone/operators/store/models.py +++ b/fiftyone/operators/store/models.py @@ -14,13 +14,10 @@ class KeyDocument(BaseModel): key: str value: Any created_at: datetime.datetime = Field( - default_factory=datetime.datetime.utcnow + default_factory=datetime.datetime.now ) updated_at: Optional[datetime.datetime] = None expires_at: Optional[datetime.datetime] = None - permissions: Optional[ - Dict[str, Any] - ] = None # Permissions can be a role/user/plugin map @staticmethod def get_expiration(ttl: Optional[int]) -> Optional[datetime.datetime]: @@ -28,7 +25,7 @@ def get_expiration(ttl: Optional[int]) -> Optional[datetime.datetime]: if ttl is None: return None - return datetime.datetime.now() + datetime.timedelta(milliseconds=ttl) + return datetime.datetime.now() + datetime.timedelta(seconds=ttl) class StoreDocument(KeyDocument): diff --git a/fiftyone/operators/store/service.py b/fiftyone/operators/store/service.py index 47dcf1ada3..f01e3b874f 100644 --- a/fiftyone/operators/store/service.py +++ b/fiftyone/operators/store/service.py @@ -39,7 +39,7 @@ def set_key(self, store_name, key, value, ttl=None): store_name: the name of the store key: the key to set value: the value to set - ttl (None): an optional TTL in milliseconds + ttl (None): an optional TTL in seconds Returns: a :class:`fiftyone.store.models.KeyDocument` @@ -68,7 +68,7 @@ def delete_key(self, store_name, key): key: the key to delete Returns: - a :class:`fiftyone.store.models.KeyDocument` + `True` if the key was deleted, `False` otherwise """ return self._repo.delete_key(store_name=store_name, key=key) diff --git a/fiftyone/operators/store/store.py b/fiftyone/operators/store/store.py index d0fc738f9f..0fee055ea6 100644 --- a/fiftyone/operators/store/store.py +++ b/fiftyone/operators/store/store.py @@ -48,7 +48,7 @@ def set(self, key: str, value: Any, ttl: Optional[int] = None) -> None: Args: key (str): The key to store the value under. value (Any): The value to store. - ttl (Optional[int], optional): The time-to-live in milliseconds. Defaults to None. + ttl (Optional[int], optional): The time-to-live in seconds. Defaults to None. """ self._store_service.set_key(self.store_name, key, value, ttl) @@ -57,8 +57,11 @@ def delete(self, key: str) -> None: Args: key (str): The key to delete. + + Returns: + bool: True if the key was deleted, False otherwise. """ - self._store_service.delete_key(self.store_name, key) + return self._store_service.delete_key(self.store_name, key) def has(self, key: str) -> bool: """Checks if the store has a specific key. diff --git a/tests/unittests/execution_store_tests.py b/tests/unittests/execution_store_tests.py index 12eb3a4c69..87d1b2f982 100644 --- a/tests/unittests/execution_store_tests.py +++ b/tests/unittests/execution_store_tests.py @@ -9,269 +9,225 @@ import time import unittest from unittest import mock -from unittest.mock import patch, MagicMock +from unittest.mock import patch, MagicMock, ANY, Mock +import datetime import fiftyone.operators as foo from fiftyone.operators.store import ExecutionStoreService from fiftyone.operators.store.models import StoreDocument, KeyDocument from fiftyone.factory.repo_factory import ExecutionStoreRepo -from fiftyone.operators.operator import Operator +from fiftyone.operators.store import ExecutionStore from fiftyone.operators.store import ExecutionStoreService +EPSILON = 0.1 -class MockOperator: - """Mock operator that interacts with ExecutionStore.""" - def __init__(self, ctx: foo.ExecutionContext): - self.ctx = ctx +class IsDateTime: + def __eq__(self, other): + return isinstance(other, datetime.datetime) - def execute(self): - # Example logic of interacting with the store - store = self.ctx.create_store("widgets") - # Set a value in the store - store.set("widget_1", {"name": "Widget One", "value": 100}, ttl=60000) +def assert_delta_seconds_approx(time_delta, seconds, epsilon=EPSILON): + assert abs(time_delta.total_seconds() - seconds) < epsilon - # Get the value back from the store - result = store.get("widget_1") - return result +class TeskKeyDocument(unittest.TestCase): + def test_get_expiration(self): + ttl = 1 + now = datetime.datetime.now() + expiration = KeyDocument.get_expiration(ttl) + time_delta = expiration - now + assert_delta_seconds_approx(time_delta, ttl) + assert isinstance(expiration, datetime.datetime) + def test_get_expiration_none(self): + ttl = None + expiration = KeyDocument.get_expiration(ttl) + assert expiration is None -class MockStoreRepo: - def __init__(self): - self.stores = {} - def create_store(self, store_name): - store = StoreDocument(store_name=store_name) - self.stores[store_name] = store - return store - - def set_key(self, store_name, key, value, ttl=None): - store = self.stores.get(store_name) - if store: - key_doc = KeyDocument(key=key, value=value, ttl=ttl) - store.keys[key] = key_doc - return key_doc - return None - - def get_key(self, store_name, key): - store = self.stores.get(store_name) - if store and key in store.keys: - return store.keys[key] - return None - - def delete_key(self, store_name, key): - store = self.stores.get(store_name) - if store and key in store.keys: - del store.keys[key] - - def delete_store(self, store_name): - if store_name in self.stores: - del self.stores[store_name] - - def update_ttl(self, store_name, key, ttl): - store = self.stores.get(store_name) - if store and key in store.keys: - store.keys[key].ttl = ttl - return store.keys[key] - return None +class ExecutionStoreServiceIntegrationTests(unittest.TestCase): + def setUp(self) -> None: + self.mock_collection = MagicMock() + self.store_repo = ExecutionStoreRepo(self.mock_collection) + self.store_service = ExecutionStoreService(self.store_repo) + def test_set_key(self): + self.store_repo.set_key( + "widgets", + "widget_1", + {"name": "Widget One", "value": 100}, + ttl=60000, + ) + self.mock_collection.update_one.assert_called_once() + self.mock_collection.update_one.assert_called_with( + {"store_name": "widgets", "key": "widget_1"}, + { + "$set": { + "store_name": "widgets", + "key": "widget_1", + "value": {"name": "Widget One", "value": 100}, + "created_at": IsDateTime(), + "updated_at": None, + "expires_at": IsDateTime(), + } + }, + upsert=True, + ) -class ExecutionStoreServiceTests(unittest.TestCase): - def setUp(self): - # Mock repository and service for testing - self.repo = MockStoreRepo() - self.svc = ExecutionStoreService(repo=self.repo) + def test_get_key(self): + self.mock_collection.find_one.return_value = { + "store_name": "widgets", + "key": "widget_1", + "value": {"name": "Widget One", "value": 100}, + "created_at": time.time(), + "updated_at": time.time(), + "expires_at": time.time() + 60000, + } + self.store_service.get_key(store_name="widgets", key="widget_1") + self.mock_collection.find_one.assert_called_once() + self.mock_collection.find_one.assert_called_with( + {"store_name": "widgets", "key": "widget_1"} + ) def test_create_store(self): - store_name = "test_store" - store = self.svc.create_store(store_name=store_name) - - self.assertIsNotNone(store) - self.assertEqual(store.store_name, store_name) - - def test_set_and_get_key(self): - store_name = "test_store" - self.svc.create_store(store_name=store_name) - - key = "test_key" - value = {"foo": "bar"} - ttl = 10000 - - key_doc = self.svc.set_key( - store_name=store_name, key=key, value=value, ttl=ttl + self.store_repo.create_store("widgets") + self.mock_collection.insert_one.assert_called_once() + self.mock_collection.insert_one.assert_called_with( + { + "store_name": "widgets", + "key": "__store__", + "value": None, + "created_at": IsDateTime(), + "updated_at": None, + "expires_at": None, + } ) - self.assertIsNotNone(key_doc) - self.assertEqual(key_doc.key, key) - self.assertEqual(key_doc.value, value) - self.assertEqual(key_doc.ttl, ttl) - - # Get the key and validate the value - fetched_key_doc = self.svc.get_key(store_name=store_name, key=key) - self.assertIsNotNone(fetched_key_doc) - self.assertEqual(fetched_key_doc.key, key) - self.assertEqual(fetched_key_doc.value, value) def test_delete_key(self): - store_name = "test_store" - self.svc.create_store(store_name=store_name) - - key = "test_key" - value = {"foo": "bar"} - self.svc.set_key(store_name=store_name, key=key, value=value) - - # Ensure the key exists - key_doc = self.svc.get_key(store_name=store_name, key=key) - self.assertIsNotNone(key_doc) - - # Delete the key - self.svc.delete_key(store_name=store_name, key=key) - - # Ensure the key is deleted - key_doc = self.svc.get_key(store_name=store_name, key=key) - self.assertIsNone(key_doc) + self.mock_collection.delete_one.return_value = Mock(deleted_count=1) + self.store_repo.delete_key("widgets", "widget_1") + self.mock_collection.delete_one.assert_called_once() + self.mock_collection.delete_one.assert_called_with( + {"store_name": "widgets", "key": "widget_1"} + ) def test_update_ttl(self): - store_name = "test_store" - self.svc.create_store(store_name=store_name) - - key = "test_key" - value = {"foo": "bar"} - self.svc.set_key(store_name=store_name, key=key, value=value) - - # Update TTL - new_ttl = 5000 - self.svc.update_ttl(store_name=store_name, key=key, new_ttl=new_ttl) + self.mock_collection.update_one.return_value = Mock(modified_count=1) + ttl_seconds = 60000 + expected_expiration = KeyDocument.get_expiration(ttl_seconds) + self.store_repo.update_ttl("widgets", "widget_1", ttl_seconds) + self.mock_collection.update_one.assert_called_once() - # Fetch key and check updated TTL - key_doc = self.svc.get_key(store_name=store_name, key=key) - self.assertIsNotNone(key_doc) - self.assertEqual(key_doc.ttl, new_ttl) + actual_call = self.mock_collection.update_one.call_args + actual_expires_at = actual_call[0][1]["$set"]["expires_at"] + time_delta = expected_expiration - actual_expires_at + assert_delta_seconds_approx(time_delta, 0, epsilon=0.0001) def test_delete_store(self): - store_name = "test_store" - self.svc.create_store(store_name=store_name) - - # Ensure the store exists - store = self.repo.stores.get(store_name) - self.assertIsNotNone(store) - - # Delete the store - self.svc.delete_store(store_name=store_name) - - # Ensure the store is deleted - store = self.repo.stores.get(store_name) - self.assertIsNone(store) - - def test_store_permissions(self): - store_name = "test_store" - self.svc.create_store(store_name=store_name) - - # Check default permissions - store = self.repo.stores.get(store_name) - self.assertIsNotNone(store) - - -class TestOperatorWithExecutionStore(Operator): - def execute(self, ctx): - # Create a store and interact with it - store = ctx.create_store("test_store") - - # Perform some store operations - store.set("my_key", {"foo": "bar"}) - value = store.get("my_key") - store.delete("my_key") + self.mock_collection.delete_many.return_value = Mock(deleted_count=1) + self.store_repo.delete_store("widgets") + self.mock_collection.delete_many.assert_called_once() + self.mock_collection.delete_many.assert_called_with( + {"store_name": "widgets"} + ) - return value + def test_list_keys(self): + self.mock_collection.find.return_value = [ + {"store_name": "widgets", "key": "widget_1"}, + {"store_name": "widgets", "key": "widget_2"}, + ] + keys = self.store_repo.list_keys("widgets") + assert keys == ["widget_1", "widget_2"] + self.mock_collection.find.assert_called_once() + self.mock_collection.find.assert_called_with({"store_name": "widgets"}) -class ExecutionStoreIntegrationTests(unittest.TestCase): - def setUp(self): - # Create a MagicMock for the MongoDB collection +class TestExecutionStoreIntegration(unittest.TestCase): + def setUp(self) -> None: self.mock_collection = MagicMock() - - # Instantiate the store repo and replace the _collection attribute self.store_repo = ExecutionStoreRepo(self.mock_collection) - - # Create the store service with the mocked repo self.store_service = ExecutionStoreService(self.store_repo) + self.store = ExecutionStore("mock_store", self.store_service) - # Create an execution context - from fiftyone.operators import ExecutionContext - - self.ctx = ExecutionContext() - self.ctx.store_service = ( - self.store_service - ) # Inject the store service into the context - - # Create an instance of the operator - self.operator = TestOperatorWithExecutionStore() - - def test_operator_execute_with_store(self): - # Mock MongoDB collection methods - self.mock_collection.update_one.return_value = None - self.mock_collection.find_one.return_value = { - "key": "my_key", - "value": {"foo": "bar"}, - } - self.mock_collection.delete_one.return_value = None - - # Call the operator's execute method - result = self.operator.execute(self.ctx) - - # Verify that the store interactions were made correctly - self.mock_collection.update_one.assert_called_once() # Checking that set operation inserts data - self.mock_collection.find_one.assert_called_once_with( - {"store_name": "test_store", "key": "my_key"} - ) # Checking that get operation retrieves data - self.mock_collection.delete_one.assert_called_once_with( - {"store_name": "test_store", "key": "my_key"} - ) # Checking that delete operation removes data - - # Verify the correct value was returned from the store - self.assertEqual(result, {"key": "my_key", "value": {"foo": "bar"}}) - - def test_operator_execute_set_key(self): - # Mock the insert_one call for the set operation - self.mock_collection.insert_one.return_value = None - - # Call the operator's execute method - self.operator.execute(self.ctx) - - # Check that insert_one (set) was called with the correct arguments - self.mock_collection.insert_one.assert_called_once_with( + def test_set(self): + self.store.set( + "widget_1", {"name": "Widget One", "value": 100}, ttl=60000 + ) + self.mock_collection.update_one.assert_called_once() + self.mock_collection.update_one.assert_called_with( + {"store_name": "mock_store", "key": "widget_1"}, { - "store_name": "test_store", - "key": "my_key", - "value": {"foo": "bar"}, - } + "$set": { + "store_name": "mock_store", + "key": "widget_1", + "value": {"name": "Widget One", "value": 100}, + "created_at": IsDateTime(), + "updated_at": None, + "expires_at": IsDateTime(), + } + }, + upsert=True, ) - def test_operator_execute_get_key(self): - # Mock the find_one call for the get operation + # def test_update(self): + # self.mock_collection.find_one.return_value = { + # "store_name": "mock_store", + # "key": "widget_1", + # "value": {"name": "Widget One", "value": 100}, + # "created_at": time.time(), + # "updated_at": time.time(), + # "expires_at": time.time() + 60000 + # } + # self.store.update_key("widget_1", {"name": "Widget One", "value": 200}) + # self.mock_collection.update_one.assert_called_once() + # self.mock_collection.update_one.assert_called_with( + # {"store_name": "mock_store", "key": "widget_1"}, + # { + # "$set": { + # "store_name": "mock_store", + # "key": "widget_1", + # "value": {"name": "Widget One", "value": 200}, + # "created_at": IsDateTime(), + # "updated_at": IsDateTime(), + # "expires_at": IsDateTime() + # } + # } + # ) + + def test_get(self): self.mock_collection.find_one.return_value = { - "key": "my_key", - "value": {"foo": "bar"}, + "store_name": "mock_store", + "key": "widget_1", + "value": {"name": "Widget One", "value": 100}, + "created_at": time.time(), + "updated_at": time.time(), + "expires_at": time.time() + 60000, } - - # Call the operator's execute method - result = self.operator.execute(self.ctx) - - # Check that find_one (get) was called correctly and returned the expected value - self.mock_collection.find_one.assert_called_once_with( - {"store_name": "test_store", "key": "my_key"} + value = self.store.get("widget_1") + assert value == {"name": "Widget One", "value": 100} + self.mock_collection.find_one.assert_called_once() + self.mock_collection.find_one.assert_called_with( + {"store_name": "mock_store", "key": "widget_1"} ) - self.assertEqual(result, {"key": "my_key", "value": {"foo": "bar"}}) - - def test_operator_execute_delete_key(self): - # Mock the delete_one call for the delete operation - self.mock_collection.delete_one.return_value = None - # Call the operator's execute method - self.operator.execute(self.ctx) + def test_list_keys(self): + self.mock_collection.find.return_value = [ + {"store_name": "mock_store", "key": "widget_1"}, + {"store_name": "mock_store", "key": "widget_2"}, + ] + keys = self.store.list_keys() + assert keys == ["widget_1", "widget_2"] + self.mock_collection.find.assert_called_once() + self.mock_collection.find.assert_called_with( + {"store_name": "mock_store"} + ) - # Check that delete_one was called with the correct arguments - self.mock_collection.delete_one.assert_called_once_with( - {"store_name": "test_store", "key": "my_key"} + def test_delete(self): + self.mock_collection.delete_one.return_value = Mock(deleted_count=1) + deleted = self.store.delete("widget_1") + assert deleted + self.mock_collection.delete_one.assert_called_once() + self.mock_collection.delete_one.assert_called_with( + {"store_name": "mock_store", "key": "widget_1"} ) From cda0bed5d571ed9a3139afb72f1f5f5f9d00e29c Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Sun, 22 Sep 2024 16:24:11 -0700 Subject: [PATCH 55/69] add missing clear store test --- fiftyone/operators/store/store.py | 2 +- tests/unittests/execution_store_tests.py | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/fiftyone/operators/store/store.py b/fiftyone/operators/store/store.py index 0fee055ea6..55ae6a15d1 100644 --- a/fiftyone/operators/store/store.py +++ b/fiftyone/operators/store/store.py @@ -76,7 +76,7 @@ def has(self, key: str) -> bool: def clear(self) -> None: """Clears all the data in the store.""" - self._store_service.clear_store(self.store_name) + self._store_service.delete_store(self.store_name) def update_ttl(self, key: str, new_ttl: int) -> None: """Updates the TTL for a specific key. diff --git a/tests/unittests/execution_store_tests.py b/tests/unittests/execution_store_tests.py index 87d1b2f982..e0cc1bc495 100644 --- a/tests/unittests/execution_store_tests.py +++ b/tests/unittests/execution_store_tests.py @@ -231,3 +231,10 @@ def test_delete(self): self.mock_collection.delete_one.assert_called_with( {"store_name": "mock_store", "key": "widget_1"} ) + + def test_clear(self): + self.store.clear() + self.mock_collection.delete_many.assert_called_once() + self.mock_collection.delete_many.assert_called_with( + {"store_name": "mock_store"} + ) From 59f054314a30a588cd92714edecadf1618b9d7ba Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Sun, 22 Sep 2024 16:43:06 -0700 Subject: [PATCH 56/69] add store listing --- fiftyone/factory/repos/execution_store.py | 7 ++++++- fiftyone/operators/store/models.py | 2 +- fiftyone/operators/store/service.py | 7 ++----- fiftyone/operators/store/store.py | 8 ++++++++ tests/unittests/execution_store_tests.py | 7 +++++++ 5 files changed, 24 insertions(+), 7 deletions(-) diff --git a/fiftyone/factory/repos/execution_store.py b/fiftyone/factory/repos/execution_store.py index 0b2e5aad9c..7f2c4aa117 100644 --- a/fiftyone/factory/repos/execution_store.py +++ b/fiftyone/factory/repos/execution_store.py @@ -1,5 +1,5 @@ """ -Execution store repository for handling storage and retrieval of stores. +Execution store repository. """ from pymongo.collection import Collection @@ -29,6 +29,11 @@ def create_store(self, store_name, permissions=None) -> StoreDocument: self._collection.insert_one(store_doc.dict()) return store_doc + def list_stores(self) -> list[str]: + """Lists all stores in the execution store.""" + # ensure that only store_name is returned, and only unique values + return self._collection.distinct("store_name") + def set_key(self, store_name, key, value, ttl=None) -> KeyDocument: """Sets or updates a key in the specified store.""" expiration = KeyDocument.get_expiration(ttl) diff --git a/fiftyone/operators/store/models.py b/fiftyone/operators/store/models.py index e05767fe22..5fd6c89e21 100644 --- a/fiftyone/operators/store/models.py +++ b/fiftyone/operators/store/models.py @@ -29,7 +29,7 @@ def get_expiration(ttl: Optional[int]) -> Optional[datetime.datetime]: class StoreDocument(KeyDocument): - """Model representing a store in the execution store.""" + """Model representing a Store.""" key: str = "__store__" value: Optional[Dict[str, Any]] = None diff --git a/fiftyone/operators/store/service.py b/fiftyone/operators/store/service.py index f01e3b874f..dc90039985 100644 --- a/fiftyone/operators/store/service.py +++ b/fiftyone/operators/store/service.py @@ -87,16 +87,13 @@ def update_ttl(self, store_name, key, new_ttl): store_name=store_name, key=key, ttl=new_ttl ) - def list_stores(self, search=None, **kwargs): + def list_stores(self): """Lists all stores matching the given criteria. - Args: - search (None): optional search term dict - Returns: a list of :class:`fiftyone.store.models.StoreDocument` """ - return self._repo.list_stores(search=search, **kwargs) + return self._repo.list_stores() def delete_store(self, store_name): """Deletes the specified store. diff --git a/fiftyone/operators/store/store.py b/fiftyone/operators/store/store.py index 55ae6a15d1..15e0ab3a01 100644 --- a/fiftyone/operators/store/store.py +++ b/fiftyone/operators/store/store.py @@ -28,6 +28,14 @@ def __init__(self, store_name: str, store_service: ExecutionStoreService): self.store_name: str = store_name self._store_service: ExecutionStoreService = store_service + def list_all_stores(self) -> list[str]: + """Lists all stores in the execution store. + + Returns: + list: A list of store names. + """ + return self._store_service.list_stores() + def get(self, key: str) -> Optional[Any]: """Retrieves a value from the store by its key. diff --git a/tests/unittests/execution_store_tests.py b/tests/unittests/execution_store_tests.py index e0cc1bc495..d5b145ba63 100644 --- a/tests/unittests/execution_store_tests.py +++ b/tests/unittests/execution_store_tests.py @@ -142,6 +142,13 @@ def test_list_keys(self): self.mock_collection.find.assert_called_once() self.mock_collection.find.assert_called_with({"store_name": "widgets"}) + def test_list_stores(self): + self.mock_collection.distinct.return_value = ["widgets", "gadgets"] + stores = self.store_repo.list_stores() + assert stores == ["widgets", "gadgets"] + self.mock_collection.distinct.assert_called_once() + self.mock_collection.distinct.assert_called_with("store_name") + class TestExecutionStoreIntegration(unittest.TestCase): def setUp(self) -> None: From 0c87c6629d92439ca18aaf92061a3f76b46f5c4b Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Sun, 22 Sep 2024 17:32:45 -0700 Subject: [PATCH 57/69] exec store cleanup and fixes --- fiftyone/factory/repos/execution_store.py | 50 ++++++++++++++++++----- tests/unittests/execution_store_tests.py | 47 +++++++-------------- 2 files changed, 54 insertions(+), 43 deletions(-) diff --git a/fiftyone/factory/repos/execution_store.py b/fiftyone/factory/repos/execution_store.py index 7f2c4aa117..69fa78b077 100644 --- a/fiftyone/factory/repos/execution_store.py +++ b/fiftyone/factory/repos/execution_store.py @@ -2,6 +2,7 @@ Execution store repository. """ +import datetime from pymongo.collection import Collection from fiftyone.operators.store.models import StoreDocument, KeyDocument @@ -35,18 +36,36 @@ def list_stores(self) -> list[str]: return self._collection.distinct("store_name") def set_key(self, store_name, key, value, ttl=None) -> KeyDocument: - """Sets or updates a key in the specified store.""" + """Sets or updates a key in the specified store""" + now = datetime.datetime.now() expiration = KeyDocument.get_expiration(ttl) key_doc = KeyDocument( - store_name=store_name, - key=key, - value=value, - expires_at=expiration if ttl else None, + store_name=store_name, key=key, value=value, updated_at=now ) - # Update or insert the key - self._collection.update_one( - _where(store_name, key), {"$set": key_doc.dict()}, upsert=True + + # Prepare the update operations + update_fields = { + "$set": key_doc.dict( + exclude={"created_at", "expires_at", "store_name", "key"} + ), + "$setOnInsert": { + "store_name": store_name, + "key": key, + "created_at": now, + "expires_at": expiration if ttl else None, + }, + } + + # Perform the upsert operation + result = self._collection.update_one( + _where(store_name, key), update_fields, upsert=True ) + + if result.upserted_id: + key_doc.created_at = now + else: + key_doc.updated_at = now + return key_doc def get_key(self, store_name, key) -> KeyDocument: @@ -57,8 +76,7 @@ def get_key(self, store_name, key) -> KeyDocument: def list_keys(self, store_name) -> list[str]: """Lists all keys in the specified store.""" - keys = self._collection.find(_where(store_name)) - # TODO: redact non-key fields + keys = self._collection.find(_where(store_name), {"key": 1}) return [key["key"] for key in keys] def update_ttl(self, store_name, key, ttl) -> bool: @@ -92,7 +110,19 @@ def __init__(self, collection: Collection): def _create_indexes(self): indices = self._collection.list_indexes() expires_at_name = "expires_at" + store_name_name = "store_name" + key_name = "key" + full_key_name = "store_name_and_key" if expires_at_name not in indices: self._collection.create_index( expires_at_name, name=expires_at_name, expireAfterSeconds=0 ) + if full_key_name not in indices: + self._collection.create_index( + [(store_name_name, 1), (key_name, 1)], + name=full_key_name, + unique=True, + ) + for name in [store_name_name, key_name]: + if name not in indices: + self._collection.create_index(name, name=name) diff --git a/tests/unittests/execution_store_tests.py b/tests/unittests/execution_store_tests.py index d5b145ba63..470d399026 100644 --- a/tests/unittests/execution_store_tests.py +++ b/tests/unittests/execution_store_tests.py @@ -64,13 +64,15 @@ def test_set_key(self): {"store_name": "widgets", "key": "widget_1"}, { "$set": { + "value": {"name": "Widget One", "value": 100}, + "updated_at": IsDateTime(), + }, + "$setOnInsert": { "store_name": "widgets", "key": "widget_1", - "value": {"name": "Widget One", "value": 100}, "created_at": IsDateTime(), - "updated_at": None, "expires_at": IsDateTime(), - } + }, }, upsert=True, ) @@ -140,7 +142,9 @@ def test_list_keys(self): keys = self.store_repo.list_keys("widgets") assert keys == ["widget_1", "widget_2"] self.mock_collection.find.assert_called_once() - self.mock_collection.find.assert_called_with({"store_name": "widgets"}) + self.mock_collection.find.assert_called_with( + {"store_name": "widgets"}, {"key": 1} + ) def test_list_stores(self): self.mock_collection.distinct.return_value = ["widgets", "gadgets"] @@ -166,42 +170,19 @@ def test_set(self): {"store_name": "mock_store", "key": "widget_1"}, { "$set": { + "updated_at": IsDateTime(), + "value": {"name": "Widget One", "value": 100}, + }, + "$setOnInsert": { "store_name": "mock_store", "key": "widget_1", - "value": {"name": "Widget One", "value": 100}, "created_at": IsDateTime(), - "updated_at": None, "expires_at": IsDateTime(), - } + }, }, upsert=True, ) - # def test_update(self): - # self.mock_collection.find_one.return_value = { - # "store_name": "mock_store", - # "key": "widget_1", - # "value": {"name": "Widget One", "value": 100}, - # "created_at": time.time(), - # "updated_at": time.time(), - # "expires_at": time.time() + 60000 - # } - # self.store.update_key("widget_1", {"name": "Widget One", "value": 200}) - # self.mock_collection.update_one.assert_called_once() - # self.mock_collection.update_one.assert_called_with( - # {"store_name": "mock_store", "key": "widget_1"}, - # { - # "$set": { - # "store_name": "mock_store", - # "key": "widget_1", - # "value": {"name": "Widget One", "value": 200}, - # "created_at": IsDateTime(), - # "updated_at": IsDateTime(), - # "expires_at": IsDateTime() - # } - # } - # ) - def test_get(self): self.mock_collection.find_one.return_value = { "store_name": "mock_store", @@ -227,7 +208,7 @@ def test_list_keys(self): assert keys == ["widget_1", "widget_2"] self.mock_collection.find.assert_called_once() self.mock_collection.find.assert_called_with( - {"store_name": "mock_store"} + {"store_name": "mock_store"}, {"key": 1} ) def test_delete(self): From 1528aea6bbc827eda3218830277504deeb01a902 Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Wed, 2 Oct 2024 08:37:03 -0700 Subject: [PATCH 58/69] refactor: remove pydantic from execution_store --- fiftyone/factory/repos/execution_store.py | 21 +++++++++++++-------- fiftyone/operators/store/models.py | 21 +++++++++++++-------- fiftyone/operators/store/store.py | 1 - 3 files changed, 26 insertions(+), 17 deletions(-) diff --git a/fiftyone/factory/repos/execution_store.py b/fiftyone/factory/repos/execution_store.py index 69fa78b077..b8fe6b786a 100644 --- a/fiftyone/factory/repos/execution_store.py +++ b/fiftyone/factory/repos/execution_store.py @@ -24,10 +24,8 @@ def __init__(self, collection: Collection): def create_store(self, store_name, permissions=None) -> StoreDocument: """Creates a store in the execution store.""" - store_doc = StoreDocument( - store_name=store_name, permissions=permissions - ) - self._collection.insert_one(store_doc.dict()) + store_doc = StoreDocument(store_name=store_name, value=permissions) + self._collection.insert_one(store_doc.to_mongo_dict()) return store_doc def list_stores(self) -> list[str]: @@ -40,14 +38,21 @@ def set_key(self, store_name, key, value, ttl=None) -> KeyDocument: now = datetime.datetime.now() expiration = KeyDocument.get_expiration(ttl) key_doc = KeyDocument( - store_name=store_name, key=key, value=value, updated_at=now + store_name=store_name, + key=key, + value=value, + updated_at=now, + expires_at=expiration, ) # Prepare the update operations update_fields = { - "$set": key_doc.dict( - exclude={"created_at", "expires_at", "store_name", "key"} - ), + "$set": { + k: v + for k, v in key_doc.to_mongo_dict().items() + if k + not in {"_id", "created_at", "expires_at", "store_name", "key"} + }, "$setOnInsert": { "store_name": store_name, "key": key, diff --git a/fiftyone/operators/store/models.py b/fiftyone/operators/store/models.py index 5fd6c89e21..f6cba9f841 100644 --- a/fiftyone/operators/store/models.py +++ b/fiftyone/operators/store/models.py @@ -1,21 +1,19 @@ -""" -Store and key models for the execution store. -""" - -from pydantic import BaseModel, Field +from dataclasses import dataclass, field, asdict from typing import Optional, Dict, Any import datetime -class KeyDocument(BaseModel): +@dataclass +class KeyDocument: """Model representing a key in the store.""" store_name: str key: str value: Any - created_at: datetime.datetime = Field( + created_at: datetime.datetime = field( default_factory=datetime.datetime.now ) + _id: Optional[Any] = None updated_at: Optional[datetime.datetime] = None expires_at: Optional[datetime.datetime] = None @@ -24,10 +22,17 @@ def get_expiration(ttl: Optional[int]) -> Optional[datetime.datetime]: """Gets the expiration date for a key with the given TTL.""" if ttl is None: return None - return datetime.datetime.now() + datetime.timedelta(seconds=ttl) + def to_mongo_dict(self, exclude_id: bool = True) -> Dict[str, Any]: + """Serializes the document to a MongoDB dictionary.""" + data = asdict(self) + if exclude_id: + data.pop("_id", None) + return data + +@dataclass class StoreDocument(KeyDocument): """Model representing a Store.""" diff --git a/fiftyone/operators/store/store.py b/fiftyone/operators/store/store.py index 15e0ab3a01..cdf761dd46 100644 --- a/fiftyone/operators/store/store.py +++ b/fiftyone/operators/store/store.py @@ -7,7 +7,6 @@ """ import logging -from fiftyone.factory.repo_factory import RepositoryFactory from fiftyone.operators.store.service import ExecutionStoreService from typing import Any, Optional From 4a45fc09da7cbd79015a0d8c6cc289dbc376a1e1 Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Thu, 3 Oct 2024 06:29:41 -0700 Subject: [PATCH 59/69] refactor: add types to all fiftyone.operator.store modules --- fiftyone/operators/store/service.py | 32 +++++++++++++++++++---------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/fiftyone/operators/store/service.py b/fiftyone/operators/store/service.py index dc90039985..2d2e1dc7b7 100644 --- a/fiftyone/operators/store/service.py +++ b/fiftyone/operators/store/service.py @@ -7,21 +7,24 @@ """ import logging +from typing import Optional, List from fiftyone.factory.repo_factory import RepositoryFactory +from fiftyone.operators.store.models import StoreDocument, KeyDocument +from fiftyone.factory.repos.execution_store import ExecutionStoreRepo logger = logging.getLogger(__name__) -class ExecutionStoreService(object): +class ExecutionStoreService: """Service for managing execution store operations.""" - def __init__(self, repo=None): + def __init__(self, repo: Optional[ExecutionStoreRepo] = None): if repo is None: repo = RepositoryFactory.execution_store_repo() - self._repo = repo + self._repo: ExecutionStoreRepo = repo - def create_store(self, store_name): + def create_store(self, store_name: str) -> StoreDocument: """Creates a new store with the specified name. Args: @@ -32,7 +35,9 @@ def create_store(self, store_name): """ return self._repo.create_store(store_name=store_name) - def set_key(self, store_name, key, value, ttl=None): + def set_key( + self, store_name: str, key: str, value: str, ttl: Optional[int] = None + ) -> KeyDocument: """Sets the value of a key in the specified store. Args: @@ -48,7 +53,7 @@ def set_key(self, store_name, key, value, ttl=None): store_name=store_name, key=key, value=value, ttl=ttl ) - def get_key(self, store_name, key): + def get_key(self, store_name: str, key: str) -> KeyDocument: """Retrieves the value of a key from the specified store. Args: @@ -60,7 +65,7 @@ def get_key(self, store_name, key): """ return self._repo.get_key(store_name=store_name, key=key) - def delete_key(self, store_name, key): + def delete_key(self, store_name: str, key: str) -> bool: """Deletes the specified key from the store. Args: @@ -72,7 +77,9 @@ def delete_key(self, store_name, key): """ return self._repo.delete_key(store_name=store_name, key=key) - def update_ttl(self, store_name, key, new_ttl): + def update_ttl( + self, store_name: str, key: str, new_ttl: int + ) -> KeyDocument: """Updates the TTL of the specified key in the store. Args: @@ -87,7 +94,7 @@ def update_ttl(self, store_name, key, new_ttl): store_name=store_name, key=key, ttl=new_ttl ) - def list_stores(self): + def list_stores(self) -> List[StoreDocument]: """Lists all stores matching the given criteria. Returns: @@ -95,7 +102,7 @@ def list_stores(self): """ return self._repo.list_stores() - def delete_store(self, store_name): + def delete_store(self, store_name: str) -> StoreDocument: """Deletes the specified store. Args: @@ -106,10 +113,13 @@ def delete_store(self, store_name): """ return self._repo.delete_store(store_name=store_name) - def list_keys(self, store_name): + def list_keys(self, store_name: str) -> List[str]: """Lists all keys in the specified store. Args: store_name: the name of the store + + Returns: + a list of keys in the store """ return self._repo.list_keys(store_name) From f4f686b6b646a0b20424288451eb9b320ac95c29 Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Thu, 3 Oct 2024 06:41:37 -0700 Subject: [PATCH 60/69] execution store cleanup --- fiftyone/operators/store/service.py | 2 +- fiftyone/operators/store/store.py | 4 ++-- tests/unittests/execution_store_tests.py | 4 +--- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/fiftyone/operators/store/service.py b/fiftyone/operators/store/service.py index 2d2e1dc7b7..6ccbd41f4f 100644 --- a/fiftyone/operators/store/service.py +++ b/fiftyone/operators/store/service.py @@ -85,7 +85,7 @@ def update_ttl( Args: store_name: the name of the store key: the key to update the TTL for - new_ttl: the new TTL in milliseconds + new_ttl: the new TTL in seconds Returns: a :class:`fiftyone.store.models.KeyDocument` diff --git a/fiftyone/operators/store/store.py b/fiftyone/operators/store/store.py index cdf761dd46..8802e3ad4e 100644 --- a/fiftyone/operators/store/store.py +++ b/fiftyone/operators/store/store.py @@ -90,7 +90,7 @@ def update_ttl(self, key: str, new_ttl: int) -> None: Args: key (str): The key to update the TTL for. - new_ttl (int): The new TTL in milliseconds. + new_ttl (int): The new TTL in seconds. """ self._store_service.update_ttl(self.store_name, key, new_ttl) @@ -101,7 +101,7 @@ def get_ttl(self, key: str) -> Optional[int]: key (str): The key to get the TTL for. Returns: - Optional[int]: The TTL in milliseconds, or None if the key does not have a TTL. + Optional[int]: The TTL in seconds, or None if the key does not have a TTL. """ return self._store_service.get_ttl(self.store_name, key) diff --git a/tests/unittests/execution_store_tests.py b/tests/unittests/execution_store_tests.py index 470d399026..b19929c6f4 100644 --- a/tests/unittests/execution_store_tests.py +++ b/tests/unittests/execution_store_tests.py @@ -8,13 +8,11 @@ import time import unittest -from unittest import mock from unittest.mock import patch, MagicMock, ANY, Mock import datetime -import fiftyone.operators as foo from fiftyone.operators.store import ExecutionStoreService -from fiftyone.operators.store.models import StoreDocument, KeyDocument +from fiftyone.operators.store.models import KeyDocument from fiftyone.factory.repo_factory import ExecutionStoreRepo from fiftyone.operators.store import ExecutionStore from fiftyone.operators.store import ExecutionStoreService From c54983865638c32c0520cfb65a2afb36fe0e6c2d Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Thu, 3 Oct 2024 06:59:29 -0700 Subject: [PATCH 61/69] remove redundant const --- fiftyone/factory/repos/execution_store.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/fiftyone/factory/repos/execution_store.py b/fiftyone/factory/repos/execution_store.py index b8fe6b786a..e58d8b1b8f 100644 --- a/fiftyone/factory/repos/execution_store.py +++ b/fiftyone/factory/repos/execution_store.py @@ -106,8 +106,6 @@ def delete_store(self, store_name) -> int: class MongoExecutionStoreRepo(ExecutionStoreRepo): """MongoDB implementation of execution store repository.""" - COLLECTION_NAME = "execution_store" - def __init__(self, collection: Collection): super().__init__(collection) self._create_indexes() From f253d638aa2bab32f2302935f1751fcf92077df3 Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Thu, 3 Oct 2024 07:02:18 -0700 Subject: [PATCH 62/69] fix store.delete() signature --- fiftyone/operators/store/store.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fiftyone/operators/store/store.py b/fiftyone/operators/store/store.py index 8802e3ad4e..0e7fd320ba 100644 --- a/fiftyone/operators/store/store.py +++ b/fiftyone/operators/store/store.py @@ -59,7 +59,7 @@ def set(self, key: str, value: Any, ttl: Optional[int] = None) -> None: """ self._store_service.set_key(self.store_name, key, value, ttl) - def delete(self, key: str) -> None: + def delete(self, key: str) -> bool: """Deletes a key from the store. Args: From 41db91ad46631e933b343403f782e71686a150d4 Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Thu, 3 Oct 2024 07:03:30 -0700 Subject: [PATCH 63/69] cleanup imports --- tests/unittests/execution_store_tests.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unittests/execution_store_tests.py b/tests/unittests/execution_store_tests.py index b19929c6f4..2ac2f3628f 100644 --- a/tests/unittests/execution_store_tests.py +++ b/tests/unittests/execution_store_tests.py @@ -15,7 +15,6 @@ from fiftyone.operators.store.models import KeyDocument from fiftyone.factory.repo_factory import ExecutionStoreRepo from fiftyone.operators.store import ExecutionStore -from fiftyone.operators.store import ExecutionStoreService EPSILON = 0.1 From 7f15101585af2533620075b24cbf376a943ac8f1 Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Thu, 17 Oct 2024 13:31:45 -0700 Subject: [PATCH 64/69] fix circular reference --- fiftyone/operators/store/service.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/fiftyone/operators/store/service.py b/fiftyone/operators/store/service.py index 6ccbd41f4f..1f35afe584 100644 --- a/fiftyone/operators/store/service.py +++ b/fiftyone/operators/store/service.py @@ -8,9 +8,7 @@ import logging from typing import Optional, List -from fiftyone.factory.repo_factory import RepositoryFactory from fiftyone.operators.store.models import StoreDocument, KeyDocument -from fiftyone.factory.repos.execution_store import ExecutionStoreRepo logger = logging.getLogger(__name__) @@ -18,7 +16,12 @@ class ExecutionStoreService: """Service for managing execution store operations.""" - def __init__(self, repo: Optional[ExecutionStoreRepo] = None): + def __init__(self, repo: Optional["ExecutionStoreRepo"] = None): + from fiftyone.factory.repo_factory import ( + RepositoryFactory, + ExecutionStoreRepo, + ) + if repo is None: repo = RepositoryFactory.execution_store_repo() From 979ea73570c211a95f2676851d8e65a97e5ea5ed Mon Sep 17 00:00:00 2001 From: brimoor Date: Mon, 21 Oct 2024 22:17:17 -0400 Subject: [PATCH 65/69] cleanup --- docs/source/plugins/developing_plugins.rst | 28 +++++++++--------- fiftyone/operators/builtin.py | 34 +++++++++++++++------- fiftyone/operators/executor.py | 25 ++++++++-------- fiftyone/operators/operations.py | 13 +++++---- 4 files changed, 59 insertions(+), 41 deletions(-) diff --git a/docs/source/plugins/developing_plugins.rst b/docs/source/plugins/developing_plugins.rst index d0769a5d18..783320f102 100644 --- a/docs/source/plugins/developing_plugins.rst +++ b/docs/source/plugins/developing_plugins.rst @@ -974,6 +974,7 @@ contains the following properties: - `ctx.dataset_name`: the name of the current dataset - `ctx.dataset` - the current |Dataset| instance - `ctx.view` - the current |DatasetView| instance +- `ctx.spaces` - the current :ref:`Spaces layout ` in the App - `ctx.current_sample` - the ID of the active sample in the App modal, if any - `ctx.selected` - the list of currently selected samples in the App, if any - `ctx.selected_labels` - the list of currently selected labels in the App, @@ -1001,7 +1002,6 @@ contains the following properties: if any - `ctx.results` - a dict containing the outputs of the `execute()` method, if it has been called -- `ctx.spaces` - The current workspace or the state of spaces in the app. - `ctx.hooks` **(JS only)** - the return value of the operator's `useHooks()` method @@ -1992,6 +1992,19 @@ subsequent sections. ctx.panel.set_state("event", "on_change_view") ctx.panel.set_data("event_data", event) + def on_change_spaces(self, ctx): + """Implement this method to set panel state/data when the current + spaces layout changes. + + The current spaces layout will be available via ``ctx.spaces``. + """ + event = { + "data": ctx.spaces, + "description": "the current spaces layout", + } + ctx.panel.set_state("event", "on_change_spaces") + ctx.panel.set_data("event_data", event) + def on_change_current_sample(self, ctx): """Implement this method to set panel state/data when a new sample is loaded in the Sample modal. @@ -2061,19 +2074,6 @@ subsequent sections. ctx.panel.set_state("event", "on_change_group_slice") ctx.panel.set_data("event_data", event) - def on_change_spaces(self, ctx): - """Implement this method to set panel state/data when the current - state of spaces changes in the app. - - The current state of spaces will be available via ``ctx.spaces``. - """ - event = { - "data": ctx.spaces, - "description": "the current state of spaces", - } - ctx.panel.set_state("event", "on_change_spaces") - ctx.panel.set_data("event_data", event) - ####################################################################### # Custom events # These events are defined by user code above and, just like builtin diff --git a/fiftyone/operators/builtin.py b/fiftyone/operators/builtin.py index c0f163f45d..57783f0842 100644 --- a/fiftyone/operators/builtin.py +++ b/fiftyone/operators/builtin.py @@ -14,6 +14,7 @@ import fiftyone.core.storage as fos import fiftyone.operators as foo import fiftyone.operators.types as types +from fiftyone.core.odm.workspace import default_workspace_factory class EditFieldInfo(foo.Operator): @@ -1957,10 +1958,11 @@ def execute(self, ctx): color = ctx.params.get("color", None) spaces = ctx.params.get("spaces", None) - if spaces is None: + curr_spaces = spaces is None + if curr_spaces: spaces = ctx.spaces else: - spaces = _parse_spaces(spaces) + spaces = _parse_spaces(ctx, spaces) ctx.dataset.save_workspace( name, @@ -1970,6 +1972,9 @@ def execute(self, ctx): overwrite=True, ) + if curr_spaces: + ctx.ops.set_spaces(name=name) + class EditWorkspaceInfo(foo.Operator): @property @@ -1995,9 +2000,14 @@ def execute(self, ctx): description = ctx.params.get("description", None) color = ctx.params.get("color", None) + curr_name = ctx.spaces.name info = dict(name=new_name, description=description, color=color) + ctx.dataset.update_workspace_info(name, info) + if curr_name is not None and curr_name != new_name: + ctx.ops.set_spaces(name=new_name) + def _edit_workspace_info_inputs(ctx, inputs): workspaces = ctx.dataset.list_workspaces() @@ -2015,12 +2025,11 @@ def _edit_workspace_info_inputs(ctx, inputs): for key in workspaces: workspace_selector.add_choice(key, label=key) - current_workspace = ctx.spaces.name if ctx.spaces else None inputs.enum( "name", workspace_selector.values(), + default=ctx.spaces.name, required=True, - default=current_workspace, label="Workspace", description="The workspace to edit", view=workspace_selector, @@ -2084,11 +2093,11 @@ def resolve_input(self, ctx): workspace_selector = types.AutocompleteView() for key in workspaces: workspace_selector.add_choice(key, label=key) - current_workspace = ctx.spaces.name if ctx.spaces else None + inputs.enum( "name", workspace_selector.values(), - default=current_workspace, + default=ctx.spaces.name, required=True, label="Workspace", description="The workspace to delete", @@ -2109,8 +2118,12 @@ def resolve_input(self, ctx): def execute(self, ctx): name = ctx.params["name"] + curr_spaces = name == ctx.spaces.name ctx.dataset.delete_workspace(name) + if curr_spaces: + ctx.ops.set_spaces(spaces=default_workspace_factory()) + class SyncLastModifiedAt(foo.Operator): @property @@ -2274,10 +2287,11 @@ def _get_non_default_frame_fields(dataset): return schema -def _parse_spaces(spaces): - if isinstance(spaces, dict): - return fo.Space.from_dict(spaces) - return fo.Space.from_json(spaces) +def _parse_spaces(ctx, spaces): + if isinstance(spaces, str): + spaces = json.loads(spaces) + + return fo.Space.from_dict(spaces) BUILTIN_OPERATORS = [ diff --git a/fiftyone/operators/executor.py b/fiftyone/operators/executor.py index 7624653a14..ac83bd3fe6 100644 --- a/fiftyone/operators/executor.py +++ b/fiftyone/operators/executor.py @@ -610,6 +610,19 @@ def has_custom_view(self): ) return has_stages or has_filters or has_extended + @property + def spaces(self): + """The current spaces layout in the FiftyOne App.""" + workspace_name = self.request_params.get("workspace_name", None) + if workspace_name is not None: + return self.dataset.load_workspace(workspace_name) + + spaces_dict = self.request_params.get("spaces", None) + if spaces_dict is not None: + return fo.Space.from_dict(spaces_dict) + + return None + @property def selected(self): """The list of selected sample IDs (if any).""" @@ -718,18 +731,6 @@ def query_performance(self): """Whether query performance is enabled.""" return self.request_params.get("query_performance", None) - @property - def spaces(self): - """The current workspace or the state of spaces in the app.""" - workspace_name = self.request_params.get("workspace_name", None) - if workspace_name is not None: - return self.dataset.load_workspace(workspace_name) - - spaces_dict = self.request_params.get("spaces", None) - if spaces_dict is None: - return None - return fo.Space.from_dict(spaces_dict) - def prompt( self, operator_uri, diff --git a/fiftyone/operators/operations.py b/fiftyone/operators/operations.py index 84c5e78e90..ef31a71c06 100644 --- a/fiftyone/operators/operations.py +++ b/fiftyone/operators/operations.py @@ -310,8 +310,9 @@ def register_panel( on_unload=None, on_change=None, on_change_ctx=None, - on_change_view=None, on_change_dataset=None, + on_change_view=None, + on_change_spaces=None, on_change_current_sample=None, on_change_selected=None, on_change_selected_labels=None, @@ -342,10 +343,12 @@ def register_panel( changes on_change_ctx (None): an operator to invoke when the panel execution context changes - on_change_view (None): an operator to invoke when the current view - changes on_change_dataset (None): an operator to invoke when the current dataset changes + on_change_view (None): an operator to invoke when the current view + changes + on_change_spaces (None): an operator to invoke when the current + spaces layout changes on_change_current_sample (None): an operator to invoke when the current sample changes on_change_selected (None): an operator to invoke when the current @@ -356,7 +359,6 @@ def register_panel( current extended selection changes on_change_group_slice (None): an operator to invoke when the group slice changes - on_change_spaces (None): an operator to invoke when the current spaces changes allow_duplicates (False): whether to allow multiple instances of the panel to the opened """ @@ -373,8 +375,9 @@ def register_panel( "on_unload": on_unload, "on_change": on_change, "on_change_ctx": on_change_ctx, - "on_change_view": on_change_view, "on_change_dataset": on_change_dataset, + "on_change_view": on_change_view, + "on_change_spaces": on_change_spaces, "on_change_current_sample": on_change_current_sample, "on_change_selected": on_change_selected, "on_change_selected_labels": on_change_selected_labels, From 4a786610115c4370713c4c8bd2317cec948899a0 Mon Sep 17 00:00:00 2001 From: brimoor Date: Mon, 21 Oct 2024 22:20:36 -0400 Subject: [PATCH 66/69] one more --- fiftyone/operators/panel.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fiftyone/operators/panel.py b/fiftyone/operators/panel.py index 1a286e044d..482519159c 100644 --- a/fiftyone/operators/panel.py +++ b/fiftyone/operators/panel.py @@ -119,14 +119,14 @@ def on_startup(self, ctx): methods = ["on_load", "on_unload", "on_change"] ctx_change_events = [ "on_change_ctx", - "on_change_view", "on_change_dataset", + "on_change_view", + "on_change_spaces", "on_change_current_sample", "on_change_selected", "on_change_selected_labels", "on_change_extended_selection", "on_change_group_slice", - "on_change_spaces", ] for method in methods + ctx_change_events: if hasattr(self, method) and callable(getattr(self, method)): From d7b47c210c954e9c3274c3b79899b6c3601bea14 Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Tue, 22 Oct 2024 09:55:26 -0700 Subject: [PATCH 67/69] frameloader fixes --- .../SchemaIO/components/FrameLoaderView.tsx | 11 ++---- fiftyone/operators/types.py | 38 +++++++++++++++++-- 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/app/packages/core/src/plugins/SchemaIO/components/FrameLoaderView.tsx b/app/packages/core/src/plugins/SchemaIO/components/FrameLoaderView.tsx index ff41d1274e..8c0e94fb1e 100644 --- a/app/packages/core/src/plugins/SchemaIO/components/FrameLoaderView.tsx +++ b/app/packages/core/src/plugins/SchemaIO/components/FrameLoaderView.tsx @@ -1,9 +1,6 @@ import React, { useEffect, useRef, useState } from "react"; import { ObjectSchemaType, ViewPropsType } from "../utils/types"; -import { - DEFAULT_FRAME_NUMBER, - GLOBAL_TIMELINE_ID, -} from "@fiftyone/playback/src/lib/constants"; +import { DEFAULT_FRAME_NUMBER } from "@fiftyone/playback/src/lib/constants"; import { BufferManager, BufferRange } from "@fiftyone/utilities"; import { usePanelEvent } from "@fiftyone/operators"; import { usePanelId, useSetPanelStateById } from "@fiftyone/spaces"; @@ -13,7 +10,7 @@ import _ from "lodash"; export default function FrameLoaderView(props: ViewPropsType) { const { schema, path, data } = props; const { view = {} } = schema; - const { on_load_range, timeline_id, target } = view; + const { on_load_range, target, timeline_name } = view; const panelId = usePanelId(); const triggerEvent = usePanelEvent(); const setPanelState = useSetPanelStateById(true); @@ -76,14 +73,14 @@ export default function FrameLoaderView(props: ViewPropsType) { [data, setPanelState, panelId, target] ); - const { isTimelineInitialized, subscribe } = useTimeline(); + const { isTimelineInitialized, subscribe } = useTimeline(timeline_name); const [subscribed, setSubscribed] = useState(false); React.useEffect(() => { if (subscribed) return; if (isTimelineInitialized) { subscribe({ - id: timeline_id || GLOBAL_TIMELINE_ID, + id: panelId, loadRange, renderFrame: myRenderFrame, }); diff --git a/fiftyone/operators/types.py b/fiftyone/operators/types.py index f861433eeb..fa5f4e0e9c 100644 --- a/fiftyone/operators/types.py +++ b/fiftyone/operators/types.py @@ -2432,13 +2432,43 @@ def to_json(self): class FrameLoaderView(View): - """Utility for loading frames and animated panels. + """Utility for animating panel state based on the given timeline_name. + + Examples:: + + def on_load(self, ctx): + panel.state.plot = { + "type": "scatter", + "x": [1, 2, 3], + "y": [1, 2, 3], + } + + def render(self, ctx): + panel.obj( + "frame_data", + view=types.FrameLoaderView( + on_load_range=self.on_load_range, + target="plot.selectedpoints", + ), + ) + panel.plot("plot") + + def load_range(self, ctx, range_to_load): + r = ctx.params.get("range") + + chunk = {} + for i in range(r[0], r[1]): + rendered_frame = [i] + chunk[f"frame_data.frames[{i}]"] = rendered_frame + + ctx.panel.set_data(chunk) + current_field = ctx.panel.state.selected_field or "default_field" + ctx.panel.set_state("frame_data.signature", current_field + str(r)) Args: - timeline_id (None): the ID of the timeline to load - on_load (None): the operator to execute when the frame is loaded - on_error (None): the operator to execute when the frame fails to load + timeline_name (None): the name of the timeline to load if provided, otherwise the default timeline on_load_range (None): the operator to execute when the frame is loading + target: the path to the property to animate """ def __init__(self, **kwargs): From cbedc2c9385999aedcd5f514f27c916d2532f987 Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Mon, 21 Oct 2024 14:02:44 -0700 Subject: [PATCH 68/69] fix overlay z-indax for panel menus --- .../plugins/SchemaIO/components/ContainerizedComponent.tsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/packages/core/src/plugins/SchemaIO/components/ContainerizedComponent.tsx b/app/packages/core/src/plugins/SchemaIO/components/ContainerizedComponent.tsx index 4caadbd7f3..78527693a3 100644 --- a/app/packages/core/src/plugins/SchemaIO/components/ContainerizedComponent.tsx +++ b/app/packages/core/src/plugins/SchemaIO/components/ContainerizedComponent.tsx @@ -7,6 +7,7 @@ import { overlayToSx, } from "../utils"; import { ViewPropsType } from "../utils/types"; +import { has } from "lodash"; export default function ContainerizedComponent(props: ContainerizedComponent) { const { schema, children } = props; @@ -22,7 +23,11 @@ export default function ContainerizedComponent(props: ContainerizedComponent) { } if (isCompositeView(schema)) { + const hasOverlay = !!schema?.view?.overlay; const sxForOverlay = overlayToSx[schema?.view?.overlay] || {}; + if (hasOverlay) { + sxForOverlay.zIndex = 999; + } return ( {containerizedChildren} From dcf166b6b2506c7bfdfbcf4d5df51cefcf0a61ab Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Tue, 22 Oct 2024 09:45:38 -0700 Subject: [PATCH 69/69] fix panel overflow --- .../plugins/SchemaIO/components/GridView.tsx | 13 ++++----- .../core/src/plugins/SchemaIO/utils/layout.ts | 27 ++++++++++++++++--- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/app/packages/core/src/plugins/SchemaIO/components/GridView.tsx b/app/packages/core/src/plugins/SchemaIO/components/GridView.tsx index 04644549bb..b6dec184d7 100644 --- a/app/packages/core/src/plugins/SchemaIO/components/GridView.tsx +++ b/app/packages/core/src/plugins/SchemaIO/components/GridView.tsx @@ -2,7 +2,7 @@ import { Box, BoxProps } from "@mui/material"; import React from "react"; import { HeaderView } from "."; import { - getAdjustedLayoutWidth, + getAdjustedLayoutDimensions, getComponentProps, getGridSx, getPath, @@ -21,12 +21,13 @@ export default function GridView(props: ViewPropsType) { const propertiesAsArray = Object.entries(properties).map(([id, property]) => { return { id, ...property }; }); - const height = props?.layout?.height as number; const parsedGap = parseGap(gap); - const width = getAdjustedLayoutWidth( - props?.layout?.width, - parsedGap - ) as number; + const { height, width } = getAdjustedLayoutDimensions({ + height: props?.layout?.height, + width: props?.layout?.width, + gap: parsedGap, + orientation, + }); const baseGridProps: BoxProps = { sx: { gap: parsedGap, ...getGridSx(view) }, diff --git a/app/packages/core/src/plugins/SchemaIO/utils/layout.ts b/app/packages/core/src/plugins/SchemaIO/utils/layout.ts index 8f5bfa35be..5801bdce83 100644 --- a/app/packages/core/src/plugins/SchemaIO/utils/layout.ts +++ b/app/packages/core/src/plugins/SchemaIO/utils/layout.ts @@ -179,11 +179,30 @@ export function parseGap(gap: number | string) { return 0; } -export function getAdjustedLayoutWidth(layoutWidth?: number, gap?: number) { - if (typeof gap === "number" && typeof layoutWidth === "number") { - return layoutWidth - gap * 8; +export function getAdjustedLayoutDimension(value?: number, gap?: number) { + if (typeof gap === "number" && typeof value === "number") { + return value - gap * 8; } - return layoutWidth; + return value; +} + +export function getAdjustedLayoutDimensions({ + height, + width, + gap, + orientation, +}: { + height?: number; + width?: number; + gap?: number; + orientation?: string; +}) { + const adjustedHeight = getAdjustedLayoutDimension(height, gap); + const adjustedWidth = getAdjustedLayoutDimension(width, gap); + if (orientation === "horizontal") { + return { height, width: adjustedWidth }; + } + return { height: adjustedHeight, width }; } type PaddingSxType = {