Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge release/v0.24.0 #4390

Merged
merged 27 commits into from
May 20, 2024
Merged

Merge release/v0.24.0 #4390

merged 27 commits into from
May 20, 2024

Conversation

benjaminpkane
Copy link
Contributor

@benjaminpkane benjaminpkane commented May 10, 2024

What changes are proposed in this pull request?

Merge release/v0.24.0 into develop

  • App: FiftyOne application changes
  • Build: Build and test infrastructure changes
  • Core: Core fiftyone Python library changes
  • Documentation: FiftyOne documentation changes
  • Other

Summary by CodeRabbit

  • New Features

    • Introduced session state refresh functionality for improved state handling and page reloads.
    • Added new method update_asset_paths to update asset paths in 3D scenes.
    • Enhanced DeepSort tracking to handle individual video samples.
  • Improvements

    • Adjusted styles in ImageContainerHeader and ViewBar for better UI consistency.
    • Improved timezone handling for date conversions.
    • Updated documentation for exporting datasets with classes and categories parameters.
    • Refined plugin descriptions and updated dataset URLs in the documentation.
  • Bug Fixes

    • Corrected typo in roles and permissions documentation.
  • Tests

    • Added new test cases for media export functionality.
    • Introduced tests for NumPy responses using server decorators.
    • Updated test setup and logic for PCD paths and dataset creation.
  • Chores

    • Various internal code refactorings for better maintainability and readability.

patrontheo and others added 23 commits April 25, 2024 00:51
fix: coco category ids can now be not sequential - avoiding memory leak
Adding custom runs plugin and minor tweaks
fix absolute css position for view bar icons
* convert to timezone only if datetime is tzaware; add exposing test
* WIP 3D export fixes and tests

* print

* errant copypasta

* move rewrite asset logic to scene_3d

* cleanup 3d import/export tests a bit
Support non-sequential COCO category IDs
* add np casting to json encoder

* isinstance

* add test

* add await
tweak zoom and workspace right alignment [v0.24.0]
* handle state payload in useRefresh

* cleanup

* skip e2e refresh

* cleanup, comment
* fix multi pcd e2e

* fix media visibility toggler e2e
Copy link
Contributor

coderabbitai bot commented May 10, 2024

Walkthrough

The updates encompass a wide array of enhancements and fixes across multiple components of the project. Key changes include modifications in the useRefresh function for better state handling, UI adjustments in various components, improvements to 3D media handling, and updates to documentation and test cases. Additionally, there are significant updates to the COCO utilities and the introduction of new methods to enhance functionality and performance.

Changes

File(s) Change Summary
app/packages/app/src/useEvents/useRefresh.ts Updated useRefresh function to handle state processing, URL resolution, and subscriptions before reloading the page.
app/packages/core/src/components/ImageContainerHeader.tsx Adjusted SliderContainer style width and padding.
app/packages/core/src/components/ViewBar/ViewBar.tsx Added top: 2px; style property to IconsContainer component.
app/packages/looker-3d/src/fo3d/Leva.tsx Added attributes to levaContainer and levaContainerHeader elements.
app/packages/looker-3d/src/fo3d/MediaTypeFo3d.tsx Updated canvasCameraProps function to handle cameraProps differently.
app/packages/operators/src/built-in-operators.ts Added LOAD_WORKSPACE_OPERATOR import and modified SetSpaces class execute method.
app/packages/spaces/src/components/Workspaces/index.tsx Changed padding-right (pr) value in Workspaces component.
docs/source/plugins/index.rst Refined plugin descriptions and introduced new plugin @voxel51/runs.
docs/source/user_guide/export_datasets.rst Updated documentation to include classes and categories parameters for export() method.
docs/source/integrations/huggingface.rst Updated dataset URL from scene_parse150 to scene_parse_150.
e2e-pw/src/oss/poms/fo3d/assets-panel/index.ts Introduced Asset3dPanelPom class with dragToToLeftCorner method.
e2e-pw/src/oss/specs/groups/modal-multi-pcd.spec.ts Modified setup of PCD paths, dataset creation, and sample addition logic.
fiftyone/core/fields.py Adjusted handling of timezones during UTC conversion for dates.
fiftyone/core/odm/database.py Modified get_async_db_conn function to check if timezone is falsy.
fiftyone/core/threed/lights.py Added name parameter to Light class and its subclasses.
fiftyone/core/threed/scene_3d.py Added update_asset_paths method to Scene3D class.
fiftyone/operators/builtin.py Modified execute method to use set_spaces method from ctx.ops.
fiftyone/operators/operations.py Added set_spaces method to class.
fiftyone/server/decorators.py Added custom JSON encoding for NumPy types and create_response function.
fiftyone/utils/coco.py Updated COCO utilities to handle classes_map and categories parameters.
fiftyone/utils/data/exporters.py Added input_to_output_paths dictionary for mapping asset paths.
fiftyone/utils/tracking/deepsort.py Refactored track method and added track_sample method in DeepSort class.
fiftyone/zoo/datasets/__init__.py Modified logic to set dataset.default_classes based on info.classes.
docs/source/teams/roles_and_permissions.rst Corrected typo for clarity in discussing user security.
tests/unittests/dataset_tests.py Introduced changes for manipulating system time within a test case.
tests/unittests/import_export_tests.py Added imports and new test cases for media export functionality.
tests/unittests/server_decorators_tests.py Introduced tests for NumPy responses using Server decorators.

🐇
In the realm of code, where changes bloom,
Functions evolve, dispelling gloom.
From states refreshed to lights that gleam,
A coder's dream, a rabbit's scheme.
With tests and docs, we pave the way,
For brighter bytes, a better day.
🐇


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

codecov bot commented May 10, 2024

Codecov Report

Attention: Patch coverage is 0% with 50 lines in your changes are missing coverage. Please review.

Project coverage is 16.00%. Comparing base (2753171) to head (efebf95).
Report is 40 commits behind head on develop.

Current head efebf95 differs from pull request most recent head d1b48f2

Please upload reports for the commit d1b48f2 to get more accurate results.

Files Patch % Lines
app/packages/app/src/useEvents/useRefresh.ts 0.00% 32 Missing ⚠️
app/packages/operators/src/built-in-operators.ts 0.00% 9 Missing ⚠️
app/packages/looker-3d/src/fo3d/Leva.tsx 0.00% 5 Missing ⚠️
...kages/core/src/components/ImageContainerHeader.tsx 0.00% 2 Missing ⚠️
...p/packages/core/src/components/ViewBar/ViewBar.tsx 0.00% 1 Missing ⚠️
...ackages/spaces/src/components/Workspaces/index.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #4390      +/-   ##
===========================================
- Coverage    16.02%   16.00%   -0.03%     
===========================================
  Files          804      807       +3     
  Lines        89221    89383     +162     
  Branches      1340     1342       +2     
===========================================
+ Hits         14300    14308       +8     
- Misses       74921    75075     +154     
Flag Coverage Δ
app 16.00% <0.00%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Out of diff range and nitpick comments (23)
fiftyone/operators/builtin.py (1)

Line range hint 53-53: Remove the unused variable sample_ids to clean up the code.

- sample_ids = ctx.dataset.add_collection(samples, new_ids=True)
fiftyone/zoo/datasets/__init__.py (2)

Line range hint 120-120: Avoid using bare except statements.

- except:
+ except Exception as e:
+     logger.error("Failed to load dataset info: %s", str(e))

Using a bare except can catch unexpected exceptions, making debugging difficult. It's better to catch specific exceptions or at least log the error for better traceability.


Line range hint 577-577: Avoid using bare except statements.

- except:
+ except Exception as e:
+     logger.error("Failed to construct zoo dataset instance: %s", str(e))

Similar to the previous comment, using a bare except can catch unexpected exceptions, which can obscure underlying issues. Catching Exception and logging the error provides more control and better error information.

fiftyone/core/odm/database.py (6)

Line range hint 281-281: Replace bare except with specific exceptions to avoid catching unexpected errors.

- except:
+ except Exception as e:
+     # Handle or log the exception e

Line range hint 291-291: Replace bare except with specific exceptions to avoid catching unexpected errors.

- except:
+ except Exception as e:
+     # Handle or log the exception e

Line range hint 616-616: Replace bare except with specific exceptions to avoid catching unexpected errors.

- except:
+ except Exception as e:
+     # Handle or log the exception e

Line range hint 861-861: Replace bare except with specific exceptions to avoid catching unexpected errors.

- except:
+ except Exception as e:
+     # Handle or log the exception e

Line range hint 1020-1020: Replace bare except with specific exceptions to avoid catching unexpected errors.

- except:
+ except Exception as e:
+     # Handle or log the exception e

Line range hint 1163-1163: Replace bare except with specific exceptions to avoid catching unexpected errors.

- except:
+ except Exception as e:
+     # Handle or log the exception e
fiftyone/utils/coco.py (7)

Line range hint 605-605: Ambiguous variable name 'l' can be confusing. Consider renaming it to a more descriptive name.


Line range hint 1795-1795: Ambiguous variable name 'l' can be confusing. Consider renaming it to a more descriptive name.


Line range hint 1820-1820: Avoid equality comparisons to True; use if include_license: for truth checks.

-    if include_license == True:
+    if include_license:

Line range hint 1984-1984: Ambiguous variable name 'l' can be confusing. Consider renaming it to a more descriptive name.


Line range hint 2127-2127: Avoid equality comparisons to True; use if extra_attrs: for truth checks.

-    if extra_attrs == True:
+    if extra_attrs:

Line range hint 2130-2130: Avoid equality comparisons to False; use if not extra_attrs: for false checks.

-    if extra_attrs == False:
+    if not extra_attrs:

Line range hint 2252-2252: Do not use bare except. Specify the exception type to handle specific exceptions and avoid catching unexpected events.

-    except:
+    except SomeSpecificException:
fiftyone/utils/data/exporters.py (7)

Line range hint 541-541: Replace bare except with specific exception handling.

-        except:
+        except Exception:

Line range hint 571-571: Replace bare except with specific exception handling.

-        except:
+        except Exception:

Line range hint 614-614: Replace bare except with specific exception handling.

-        except:
+        except Exception:

Line range hint 649-649: Replace bare except with specific exception handling.

-        except:
+        except Exception:

Line range hint 904-904: Replace bare except with specific exception handling.

-        except:
+        except Exception:

Line range hint 3821-3821: Use direct truth checks instead of inequality comparisons to False.

-    if include_confidence == True:
+    if include_confidence:

Line range hint 3947-3947: Use direct truth checks instead of equality comparisons to True.

-    if include_confidence == True:
+    if include_confidence:
Review Details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits Files that changed from the base of the PR and between 032220e and efebf95.
Files selected for processing (24)
  • app/packages/app/src/useEvents/useRefresh.ts (1 hunks)
  • app/packages/core/src/components/ImageContainerHeader.tsx (1 hunks)
  • app/packages/core/src/components/ViewBar/ViewBar.tsx (1 hunks)
  • app/packages/looker-3d/src/fo3d/Leva.tsx (1 hunks)
  • app/packages/operators/src/built-in-operators.ts (2 hunks)
  • app/packages/spaces/src/components/Workspaces/index.tsx (1 hunks)
  • docs/source/plugins/index.rst (2 hunks)
  • docs/source/user_guide/export_datasets.rst (1 hunks)
  • e2e-pw/src/oss/poms/fo3d/assets-panel/index.ts (1 hunks)
  • e2e-pw/src/oss/specs/groups/modal-multi-pcd.spec.ts (2 hunks)
  • e2e-pw/src/oss/specs/smoke-tests/quickstart-groups.spec.ts (4 hunks)
  • fiftyone/core/fields.py (1 hunks)
  • fiftyone/core/odm/database.py (1 hunks)
  • fiftyone/core/threed/scene_3d.py (1 hunks)
  • fiftyone/operators/builtin.py (1 hunks)
  • fiftyone/operators/operations.py (2 hunks)
  • fiftyone/server/decorators.py (3 hunks)
  • fiftyone/utils/coco.py (42 hunks)
  • fiftyone/utils/data/exporters.py (3 hunks)
  • fiftyone/utils/tracking/deepsort.py (2 hunks)
  • fiftyone/zoo/datasets/init.py (1 hunks)
  • tests/unittests/dataset_tests.py (2 hunks)
  • tests/unittests/import_export_tests.py (3 hunks)
  • tests/unittests/server_decorators_tests.py (1 hunks)
Files skipped from review due to trivial changes (4)
  • app/packages/core/src/components/ImageContainerHeader.tsx
  • app/packages/core/src/components/ViewBar/ViewBar.tsx
  • app/packages/looker-3d/src/fo3d/Leva.tsx
  • app/packages/spaces/src/components/Workspaces/index.tsx
Additional Context Used
Ruff (35)
fiftyone/core/odm/database.py (6)

281-281: Do not use bare except


291-291: Do not use bare except


616-616: Do not use bare except


861-861: Do not use bare except


1020-1020: Do not use bare except


1163-1163: Do not use bare except

fiftyone/operators/builtin.py (1)

53-53: Local variable sample_ids is assigned to but never used

fiftyone/utils/coco.py (8)

605-605: Ambiguous variable name: l


917-917: Ambiguous variable name: l


1795-1795: Ambiguous variable name: l


1820-1820: Avoid equality comparisons to True; use if include_license: for truth checks


1984-1984: Ambiguous variable name: l


2127-2127: Avoid equality comparisons to True; use if extra_attrs: for truth checks


2130-2130: Avoid equality comparisons to False; use if not extra_attrs: for false checks


2252-2252: Do not use bare except

fiftyone/utils/data/exporters.py (9)

541-541: Do not use bare except


571-571: Do not use bare except


614-614: Do not use bare except


649-649: Do not use bare except


904-904: Do not use bare except


3821-3821: Avoid inequality comparisons to False; use if include_confidence: for truth checks


3821-3821: Avoid inequality comparisons to False; use if include_attributes: for truth checks


3947-3947: Avoid equality comparisons to True; use if include_confidence: for truth checks


3952-3952: Avoid equality comparisons to True; use if include_attributes: for truth checks

fiftyone/zoo/datasets/__init__.py (2)

120-120: Do not use bare except


577-577: Do not use bare except

tests/unittests/dataset_tests.py (3)

312-312: Local variable view is assigned to but never used


1305-1305: Do not assign a lambda expression, use a def


1489-1489: Do not assign a lambda expression, use a def

tests/unittests/import_export_tests.py (6)

2162-2162: pydicom imported but unused


3600-3600: Local variable img_filepath is assigned to but never used


3617-3617: Local variable img_filepath is assigned to but never used


3635-3635: Local variable img_filepath is assigned to but never used


3664-3664: Local variable img_filepath is assigned to but never used


3734-3734: Local variable vid_filepath is assigned to but never used

Path-based Instructions (5)
e2e-pw/src/oss/poms/fo3d/assets-panel/index.ts (1)

Pattern **/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.

app/packages/app/src/useEvents/useRefresh.ts (1)

Pattern **/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.

e2e-pw/src/oss/specs/groups/modal-multi-pcd.spec.ts (1)

Pattern **/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.

e2e-pw/src/oss/specs/smoke-tests/quickstart-groups.spec.ts (1)

Pattern **/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.

app/packages/operators/src/built-in-operators.ts (1)

Pattern **/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.

Additional comments not posted (53)
e2e-pw/src/oss/poms/fo3d/assets-panel/index.ts (1)

12-35: Well-implemented method for dragging the 3D assets panel.

The method dragToToLeftCorner correctly handles visibility checks, error scenarios, and performs the drag operation using calculated offsets. Good use of asynchronous operations and error handling.

app/packages/app/src/useEvents/useRefresh.ts (1)

12-34: Proper use of React and Recoil patterns in useRefresh hook.

The useRefresh hook correctly uses useCallback to memoize the refresh function, handles state processing, URL resolution, and navigation. It also properly manages subscriptions and state updates to trigger re-renders. Good adherence to React and Recoil best practices.

fiftyone/server/decorators.py (1)

Line range hint 35-54: Well-implemented server utilities for response handling and routing.

The create_response function and route decorator are correctly implemented with comprehensive error handling, asynchronous operations, and custom JSON encoding for numpy types. Good use of modern Python features and best practices in web server development.

e2e-pw/src/oss/specs/groups/modal-multi-pcd.spec.ts (1)

Line range hint 20-69: Effective end-to-end testing of modal interactions with PCD files.

The test scenarios in modal-multi-pcd.spec.ts effectively set up PCD files, add them to a dataset, and verify the modal's behavior when interacting with 3D slices. Good use of assertions to check UI states and proper handling of dynamic file paths in the setup.

e2e-pw/src/oss/specs/smoke-tests/quickstart-groups.spec.ts (1)

Line range hint 17-106: Comprehensive smoke testing of UI interactions in "quickstart-groups".

The smoke tests in quickstart-groups.spec.ts effectively cover a range of UI interactions, including sample navigation, media visibility toggling, and group filtering. Good use of the Asset3dPanelPom for adjusting UI elements and robust assertions to verify the UI states.

fiftyone/utils/tracking/deepsort.py (1)

Line range hint 25-164: Robust implementation of object tracking with DeepSort.

The DeepSort class provides well-implemented methods for setting up and performing object tracking in video streams. The methods are well-documented, handle errors gracefully, and use modern Python practices for handling video data and tracking logic.

docs/source/plugins/index.rst (1)

Line range hint 1-114: Clear and informative documentation for the FiftyOne plugins framework.

The documentation in index.rst effectively provides an overview of the FiftyOne plugins framework, detailing available plugins and their functionalities. The structure is clear, and the use of external links and tables enhances the readability and utility of the documentation.

fiftyone/operators/operations.py (1)

320-333: Ensure proper handling of spaces and name parameters in set_spaces.

fiftyone/core/threed/scene_3d.py (1)

262-300: Review the handling of asset paths in update_asset_paths to ensure all paths are correctly updated.

app/packages/operators/src/built-in-operators.ts (1)

775-782: Ensure that the LOAD_WORKSPACE_OPERATOR is correctly used in SetSpaces and that error handling for missing parameters is robust.

Verification successful

The search results confirm that the LOAD_WORKSPACE_OPERATOR is defined in app/packages/spaces/src/components/Workspaces/constants.ts and is used in multiple files within the codebase, including the built-in-operators.ts file where it is used in the SetSpaces class as described in the review comment. This indicates that the operator is implemented and utilized correctly in the context of the SetSpaces class.

Additionally, the error handling for missing parameters in the SetSpaces class, which throws an error if neither spaces nor name is provided, appears to be robust and correctly implemented.

Based on this information, the review comment concerning the correct usage of LOAD_WORKSPACE_OPERATOR and the error handling in the SetSpaces class is verified as accurate.

  • The LOAD_WORKSPACE_OPERATOR is correctly imported and used in app/packages/operators/src/built-in-operators.ts.
  • The error handling for missing parameters in the SetSpaces class is implemented as expected.
Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify that the `LOAD_WORKSPACE_OPERATOR` is correctly implemented and used.

# Test: Search for the implementation and usage of `LOAD_WORKSPACE_OPERATOR`. Expect: Correct implementation and usage.
rg --type typescript $'LOAD_WORKSPACE_OPERATOR'

Length of output: 81



Script:

#!/bin/bash
# Description: Re-verify the implementation and usage of `LOAD_WORKSPACE_OPERATOR` by searching in TypeScript files.

# Adjusted Test: Search for the implementation and usage of `LOAD_WORKSPACE_OPERATOR` in `.ts` and `.tsx` files. Expect: Correct implementation and usage.
rg 'LOAD_WORKSPACE_OPERATOR' --glob '*.ts' --glob '*.tsx'

Length of output: 973

fiftyone/zoo/datasets/__init__.py (8)

Line range hint 35-71: Function list_zoo_datasets implementation looks good.

The function effectively utilizes helper functions to filter datasets based on tags and sources. The logic is clear and well-documented.


Line range hint 73-95: Function list_downloaded_zoo_datasets implementation looks good.

The function handles potential errors during directory listing and iterates over subdirectories to gather information about downloaded datasets. The error handling and logic are appropriate.


Line range hint 97-160: Function download_zoo_dataset implementation looks good.

The function is complex but well-structured, using helper functions to parse dataset details and handle the download process. The documentation is thorough, and the function handles different scenarios like existing splits and overwrites effectively.


Line range hint 162-364: Function load_zoo_dataset implementation looks good.

This function is well-documented and handles a variety of scenarios including downloading datasets if necessary, loading existing datasets, and managing data importers. The use of helper functions to extract arguments and manage dataset importers is effective and maintains code clarity.


Line range hint 366-402: Function find_zoo_dataset implementation looks good.

The function effectively finds the directory of a specified dataset and handles cases where the dataset or split is not downloaded. The error handling and use of helper functions to parse dataset details are appropriate.


Line range hint 404-424: Function load_zoo_dataset_info implementation looks good.

The function handles loading of dataset information effectively, including error handling for cases where the dataset is not downloaded. The use of helper functions to parse dataset details maintains code clarity.


Line range hint 426-450: Function get_zoo_dataset implementation looks good.

The function retrieves a dataset instance and handles errors during construction effectively. The error handling and use of helper functions to retrieve the dataset class are appropriate.


Line range hint 452-474: Function delete_zoo_dataset implementation looks good.

The function effectively deletes a specified dataset or split from the local disk. It uses helper functions to find the dataset or split directory and updates the dataset information if a split is deleted.

fiftyone/core/fields.py (1)

519-526: The adjustments to ensure UTC timezone handling in the DateField class are correctly implemented and follow best practices for timezone consistency.

tests/unittests/import_export_tests.py (9)

4681-4684: The class ThreeDMediaTests is well-documented, providing a clear description of its purpose.


4686-4707: The method _build_flat_relative correctly constructs a dataset with relative paths. It properly handles the creation of assets and their addition to the dataset.


4709-4736: The method _build_flat_absolute is implemented correctly, handling absolute paths for assets and ensuring they are written correctly.


4741-4800: The method _build_nested_relative effectively handles the creation of a nested dataset structure with relative paths. It ensures that assets are placed correctly in the nested directories.


4802-4808: The method _assert_scene_content correctly asserts the content of the scene files, ensuring that the read content matches the expected filenames.


4811-4834: The method test_flat_relative effectively tests the export functionality for a flat dataset with relative paths, ensuring that all files are exported correctly and that the scene content remains consistent.


4836-4865: The method test_flat_absolute correctly tests the export functionality for a flat dataset with absolute paths, ensuring that all files are exported correctly and that the scene content remains consistent.


4867-4942: The method test_relative_nested_flatten effectively tests the export functionality for a nested dataset with relative paths, ensuring that the structure is flattened during export and that duplicate filenames are handled correctly.


4943-4984: The method test_relative_nested_maintain correctly tests the export functionality for maintaining the nested directory structure during export, ensuring that the relative paths in the .fo3d files are preserved and that the file contents are correct.

tests/unittests/dataset_tests.py (25)

Line range hint 29-48: The implementation of test_list_datasets looks good and covers the expected functionality effectively.


Line range hint 50-84: The implementation of test_dataset_names correctly tests dataset naming and slug functionalities.


Line range hint 86-112: The implementation of test_delete_dataset correctly tests the dataset deletion functionality.


Line range hint 114-120: The implementation of test_backing_doc_class correctly tests the backing document class functionality.


Line range hint 122-138: The implementation of test_eq correctly tests the equality of dataset instances.


Line range hint 140-156: The implementation of test_last_loaded_at correctly tests the updating of the last_loaded_at attribute.


Line range hint 158-182: The implementation of test_dataset_tags correctly tests the handling of dataset tags.


Line range hint 184-196: The implementation of test_dataset_description correctly tests the handling of the description attribute.


Line range hint 198-216: The implementation of test_dataset_info correctly tests the handling of the info attribute.


Line range hint 218-286: The implementation of test_dataset_field_metadata correctly tests the handling of field metadata.


Line range hint 288-324: The implementation of test_dataset_app_config correctly tests the handling of the app configuration.


Line range hint 326-352: The implementation of test_meta_dataset correctly tests various operations on a meta dataset.


Line range hint 354-394: The implementation of test_indexes correctly tests the handling of indexes in a dataset.


Line range hint 396-418: The implementation of test_iter_samples correctly tests the iteration over samples in a dataset.


Line range hint 420-577: The implementation of test_date_fields correctly tests the handling of date fields in a dataset, including timezone changes.


Line range hint 579-606: The implementation of test_datetime_fields correctly tests the handling of datetime fields in a dataset, including timezone conversions.


Line range hint 608-688: The implementation of test_get_field correctly tests the retrieval of various types of fields from a dataset.


Line range hint 690-770: The implementation of test_get_field_frames correctly tests the retrieval of various types of frame fields from a dataset.


Line range hint 772-792: The implementation of test_field_names correctly tests the naming rules for fields in a dataset.


Line range hint 794-814: The implementation of test_frame_field_names correctly tests the naming rules for frame fields in a dataset, including handling of reserved keywords.


Line range hint 816-880: The implementation of test_field_schemas correctly tests the retrieval of field schemas from a dataset based on various criteria.


Line range hint 882-946: The implementation of test_frame_field_schemas correctly tests the retrieval of frame field schemas from a dataset based on various criteria.


Line range hint 948-972: The implementation of test_add_list_subfield correctly tests the addition of subfields to list fields in a dataset.


Line range hint 974-994: The implementation of test_one correctly tests the retrieval of a single sample from a dataset based on a filter.


Line range hint 996-1034: The implementation of test_merge_sample correctly tests the merging of sample data in a dataset.

Comment on lines +198 to +202
if isinstance(classes, dict):
classes_map = classes
else:
classes_map = {i: label for i, label in enumerate(classes)}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using a dictionary comprehension for clarity and efficiency.

-        classes_map = {i: label for i, label in enumerate(classes)}
+        classes_map = dict(enumerate(classes))

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
if isinstance(classes, dict):
classes_map = classes
else:
classes_map = {i: label for i, label in enumerate(classes)}
if isinstance(classes, dict):
classes_map = classes
else:
classes_map = dict(enumerate(classes))

"name": l,
"supercategory": None,
}
for i, l in enumerate(classes)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ambiguous variable name 'l' can be confusing. Consider renaming it to a more descriptive name.

Comment on lines +16 to +32
class TestNumPyResponse(unittest.IsolatedAsyncioTestCase):
async def test_numpy_response(self):
await create_response(
{
"float16": np.array([16.0], dtype=np.float16),
"float32": np.array([32.0], dtype=np.float32),
"float64": np.array([64.0], dtype=np.float64),
"int8": np.array([8], dtype=np.int8),
"int16": np.array([8], dtype=np.int16),
"int32": np.array([8], dtype=np.int32),
"int64": np.array([8], dtype=np.int64),
"uint8": np.array([8], dtype=np.uint8),
"uint6": np.array([8], dtype=np.uint16),
"uint32": np.array([8], dtype=np.uint32),
"uint64": np.array([8], dtype=np.uint64),
}
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add assertions to verify the output of create_response.

The test method test_numpy_response currently does not include any assertions to verify the output of the create_response function. It's crucial to add assertions to ensure that the function behaves as expected with various numpy data types.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Review Details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits Files that changed from the base of the PR and between efebf95 and 1712d37.
Files selected for processing (1)
  • docs/source/integrations/huggingface.rst (1 hunks)
Files skipped from review due to trivial changes (1)
  • docs/source/integrations/huggingface.rst

* bug: use name arg in lights

* add null checking when computing camera props

* upgrade three.js deps

* default name arg to class name in base class
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Review Details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits Files that changed from the base of the PR and between 1712d37 and 06df8bb.
Files ignored due to path filters (2)
  • app/packages/looker-3d/package.json is excluded by !**/*.json
  • app/yarn.lock is excluded by !**/*.lock, !**/*.lock
Files selected for processing (2)
  • app/packages/looker-3d/src/fo3d/MediaTypeFo3d.tsx (1 hunks)
  • fiftyone/core/threed/lights.py (15 hunks)
Files not reviewed due to errors (1)
  • fiftyone/core/threed/lights.py (no review received)
Additional Context Used
Path-based Instructions (1)
app/packages/looker-3d/src/fo3d/MediaTypeFo3d.tsx (1)

Pattern **/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Review Details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits Files that changed from the base of the PR and between 06df8bb and d1b48f2.
Files selected for processing (1)
  • docs/source/teams/roles_and_permissions.rst (1 hunks)
Files skipped from review due to trivial changes (1)
  • docs/source/teams/roles_and_permissions.rst

Copy link
Contributor

@imanjra imanjra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@benjaminpkane benjaminpkane merged commit 44675b2 into develop May 20, 2024
18 checks passed
@benjaminpkane benjaminpkane deleted the release/v0.24.0 branch May 20, 2024 14:47
@benjaminpkane benjaminpkane restored the release/v0.24.0 branch May 20, 2024 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants