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

Adding created_at, last_modified_at, and read-only fields #4597

Merged
merged 15 commits into from
Sep 15, 2024

Conversation

brimoor
Copy link
Contributor

@brimoor brimoor commented Jul 26, 2024

Change log

  • Adds support for marking fields as read-only
  • Adds auto-populated (and read-only) created_at fields to Sample and Frame
  • Adds auto-populated (and read-only) last_modified_at fields to Sample and Frame
  • Adds auto-populated Dataset.last_modified_at property
  • Adds auto-populated Field.created_at property
  • Adds auto-populated created_at property to all fields

Implementation details

As currently implemented, Dataset.last_modified_at is not automatically updated in response to changes to its child samples, and Sample.last_modified_at of video samples is not automatically updated in response to modifications to its child frames. Instead, the user must cause this to happen (if they want it) by calling:

dataset.sync_last_modified_at()

I chose this path because:

  • created_at and last_modified_at indexes are created by default (with the assumption that sorting by these fields will be popular), so computing $max across these collections is efficient
  • automatically updating upstream last_modified_at values upon every downstream update would be a significant performance penalty as it would 2x or 3x (for frame modifications) the number of update operations that must be performed.

Migration considerations

The upward migration declares created_at and last_modified_at fields on all datasets and populates the created_at values via {"$toDate": "$_id"}, taking advantage of the fact that ObjectIds encode the original timestamp when they were generated. Since we do not know when the samples were actually last modified, though, last_modified_at is left as None (okay we could also set this to {"$toDate": "$_id"} as well since we technically edited the sample by setting created_at, but that's not any more informative than leaving it as None). I ensured these None values will not cause problems when edits are later made or sync_last_modified_at() is later called.

The downward migration does not delete the created_at and last_modified_at fields, since they are still valid fields, they will just no longer be default + read-only fields and the last_modified_at field will no longer be automatically updated. However, the assumption is that the user will either want to re-upgrade soon, at which point they'll be glad to have their last_modified_at values retained, or else they can just use delete_sample_field() to manually delete these fields.

Example usage

created_at/last_modified_at

import fiftyone as fo
import fiftyone.zoo as foz

dataset = foz.load_zoo_dataset("quickstart")

print(dataset.created_at)
# 2024-07-26 04:45:56.062000

print(dataset.last_modified_at)
# 2024-07-26 04:45:56.373000

print(dataset.bounds("created_at"))
# (datetime.datetime(2024, 7, 26, 4, 45, 56, 386000),
#  datetime.datetime(2024, 7, 26, 4, 45, 59, 901000))

print(dataset.bounds("last_modified_at"))
# (datetime.datetime(2024, 7, 26, 4, 45, 56, 386000),
#  datetime.datetime(2024, 7, 26, 4, 45, 59, 901000))

dataset.set_values("foo", ["bar"] * len(dataset))

print(dataset.bounds("last_modified_at"))
# (datetime.datetime(2024, 7, 26, 4, 46, 0, 36000),
#  datetime.datetime(2024, 7, 26, 4, 46, 0, 36000))

for sample in dataset.select_fields():
    sample["spam"] = "eggs"
    sample.save()

print(dataset.bounds("last_modified_at"))
# (datetime.datetime(2024, 7, 26, 4, 46, 0, 107000),
#  datetime.datetime(2024, 7, 26, 4, 46, 2, 140000))

dataset.sync_last_modified_at()

print(dataset.last_modified_at)
# 2024-07-26 04:46:02.140000
from datetime import datetime

sample = dataset.first()

sample.created_at = datetime.utcnow()
# ValueError: Cannot edit read-only field 'created_at'

sample.last_modified_at = datetime.utcnow()
# ValueError: Cannot edit read-only field 'last_modified_at'

Read-only fields

sample = dataset.first()

field1 = dataset.get_field("filepath")
field1.read_only = True
field1.save()

field2 = dataset.get_field("ground_truth.detections.label")
field2.read_only = True
field2.save()

sample.filepath = "no.jpg"
# ValueError: Cannot edit read-only field 'filepath'

sample.ground_truth.detections[0].label = "no"
sample.save()
# ValueError: Cannot edit read-only field 'ground_truth.detections.label'

field1.read_only = False
field1.save()

field2.read_only = False
field2.save()

sample.filepath = "yes.jpg"
sample.ground_truth.detections[0].label = "yes"
sample.save()  # success

@brimoor brimoor added the feature Work on a feature request label Jul 26, 2024
@brimoor brimoor requested a review from swheaton July 26, 2024 05:00
Copy link
Contributor

coderabbitai bot commented Jul 26, 2024

Walkthrough

Significant updates have been made to the FiftyOne framework, focusing on frame metadata handling and improved dataset management. Enhancements include new parameters for better timestamp tracking, refined validation processes, a migration script for version 0.26.0, and version updates across related modules, all aimed at improving dataset integrity and usability.

Changes

Files Change Summary
fiftyone/core/frame.py Enhanced frame metadata handling by adding new parameters and improving validation logic.
fiftyone/migrations/revisions/v0_26_0.py Introduced a migration script to implement dataset timestamp management for version 0.26.0.
package/desktop/setup.py, setup.py Updated version numbers to reflect new releases and adjusted dependencies accordingly.
fiftyone/core/collections.py, fiftyone/core/dataset.py Enhanced dataset management with timestamp tracking and improved handling of read-only fields.
fiftyone/core/fields.py, fiftyone/core/odm/dataset.py Added read_only fields and improved field validation and metadata management.
tests/unittests/dataset_tests.py Introduced tests for timestamp management and read-only field behavior.

Poem

🐇 In the code where frames do play,
New timestamps shine through night and day.
With validation strong and true,
Our datasets dance, refreshed anew!
Let's hop along and celebrate,
For every change we cultivate! 🎉


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.
    • @coderabbitai help me debug CodeRabbit configuration file.

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 using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

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.

if sets:
updates["$set"] = sets
try:
# OPTIMIZATION: we monkey patch `to_mongo()` and
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apologies for this, I couldn't help myself...

This significantly improves the efficiency of sample.save() if a sample contains many large fields but you've only made a simple edit to one field.

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: 4

Outside diff range, codebase verification and nitpick comments (4)
fiftyone/core/frame.py (1)

798-824: Improve exception handling.

Consider using raise ... from err or raise ... from None to distinguish exceptions from errors in exception handling.

-  except Exception as e:
+  except Exception as e:
+      raise ValueError(
+          "Invalid value for field '%s' of frame %d. "
+          "Reason: %s"
+          % (field_name, frame.frame_number, str(e))
+      ) from e
Tools
Ruff

814-818: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

fiftyone/core/fields.py (2)

Line range hint 59-111: Simplify nested if statements.

The nested if statements can be combined for better readability.

-  if read_only is not None:
-      if not isinstance(read_only, bool):
-          raise ValueError("read_only must be a boolean")
+  if read_only is not None and not isinstance(read_only, bool):
+      raise ValueError("read_only must be a boolean")

Line range hint 152-214: Simplify nested if statements.

The nested if statements can be combined for better readability.

-  if read_only is not None:
-      if read_only != field.read_only:
-          raise ValueError(
-              "%s has read_only %s, not %s"
-              % (_make_prefix(path), field.read_only, read_only)
-          )
+  if read_only is not None and read_only != field.read_only:
+      raise ValueError(
+          "%s has read_only %s, not %s"
+          % (_make_prefix(path), field.read_only, read_only)
+      )
tests/unittests/import_export_tests.py (1)

3622-3622: Ensure consistency in test assertions

The new assertions for created_at and last_modified_at are present in tests/unittests/import_export_tests.py but are missing in the following files:

  • tests/unittests/group_tests.py
  • tests/intensive/import_export_tests.py

Please add the new assertions to these files to ensure consistency in testing the test_legacy_fiftyone_dataset function.

Analysis chain

LGTM! But verify the function usage in the codebase.

The code changes are approved.

However, ensure that all function calls to test_legacy_fiftyone_dataset are updated to include the new assertions.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `test_legacy_fiftyone_dataset` include the new assertions.

# Test: Search for the function usage. Expect: Only occurances of the new assertions.
rg --type python -A 5 $'test_legacy_fiftyone_dataset'

Length of output: 83


Script:

#!/bin/bash
# Description: Verify all function calls to `test_legacy_fiftyone_dataset` include the new assertions.

# Test: Search for the function usage. Expect: Only occurrences of the new assertions.
rg --type py -A 5 $'test_legacy_fiftyone_dataset'

Length of output: 2156


Script:

#!/bin/bash
# Description: Verify the presence of the new assertions in the identified files.

# Check for `created_at` and `last_modified_at` in `tests/unittests/group_tests.py`
rg --type py -A 5 'created_at' tests/unittests/group_tests.py

# Check for `created_at` and `last_modified_at` in `tests/unittests/import_export_tests.py`
rg --type py -A 5 'created_at' tests/unittests/import_export_tests.py

# Check for `created_at` and `last_modified_at` in `tests/intensive/import_export_tests.py`
rg --type py -A 5 'created_at' tests/intensive/import_export_tests.py

Length of output: 3171

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 53c4466 and 2bf6843.

Files selected for processing (37)
  • docs/source/cheat_sheets/views_cheat_sheet.rst (5 hunks)
  • docs/source/integrations/coco.rst (1 hunks)
  • docs/source/user_guide/app.rst (1 hunks)
  • docs/source/user_guide/basics.rst (6 hunks)
  • docs/source/user_guide/evaluation.rst (1 hunks)
  • docs/source/user_guide/groups.rst (15 hunks)
  • docs/source/user_guide/using_datasets.rst (39 hunks)
  • docs/source/user_guide/using_views.rst (9 hunks)
  • fiftyone/core/clips.py (4 hunks)
  • fiftyone/core/collections.py (33 hunks)
  • fiftyone/core/dataset.py (67 hunks)
  • fiftyone/core/document.py (3 hunks)
  • fiftyone/core/fields.py (48 hunks)
  • fiftyone/core/frame.py (10 hunks)
  • fiftyone/core/odm/dataset.py (5 hunks)
  • fiftyone/core/odm/document.py (11 hunks)
  • fiftyone/core/odm/embedded_document.py (2 hunks)
  • fiftyone/core/odm/frame.py (2 hunks)
  • fiftyone/core/odm/mixins.py (31 hunks)
  • fiftyone/core/odm/sample.py (2 hunks)
  • fiftyone/core/odm/utils.py (5 hunks)
  • fiftyone/core/patches.py (2 hunks)
  • fiftyone/core/stages.py (9 hunks)
  • fiftyone/core/video.py (3 hunks)
  • fiftyone/core/view.py (10 hunks)
  • fiftyone/migrations/revisions/v0_25_0.py (1 hunks)
  • fiftyone/utils/data/importers.py (5 hunks)
  • package/desktop/setup.py (1 hunks)
  • setup.py (2 hunks)
  • tests/unittests/dataset_tests.py (48 hunks)
  • tests/unittests/group_tests.py (8 hunks)
  • tests/unittests/import_export_tests.py (3 hunks)
  • tests/unittests/index_tests.py (4 hunks)
  • tests/unittests/patches_tests.py (6 hunks)
  • tests/unittests/run_tests.py (1 hunks)
  • tests/unittests/video_tests.py (22 hunks)
  • tests/unittests/view_tests.py (6 hunks)
Files skipped from review due to trivial changes (2)
  • package/desktop/setup.py
  • setup.py
Additional context used
Ruff
fiftyone/core/odm/dataset.py

9-9: datetime.datetime imported but unused

Remove unused import: datetime.datetime

(F401)

fiftyone/core/frame.py

814-818: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

fiftyone/core/fields.py

107-108: Use a single if statement instead of nested if statements

Combine if statements using and

(SIM102)


208-209: Use a single if statement instead of nested if statements

Combine if statements using and

(SIM102)

fiftyone/core/odm/mixins.py

10-10: itertools imported but unused

Remove unused import: itertools

(F401)


122-124: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)


296-296: Use key in dict instead of key in dict.keys()

Remove .keys()

(SIM118)

tests/unittests/dataset_tests.py

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

Rewrite key_fcn as a def

(E731)


1584-1584: Found useless expression. Either assign it to a variable or remove it.

(B018)


1586-1586: Found useless expression. Either assign it to a variable or remove it.

(B018)


2756-2756: Found useless expression. Either assign it to a variable or remove it.

(B018)


2767-2767: Found useless expression. Either assign it to a variable or remove it.

(B018)


3047-3047: Found useless expression. Either assign it to a variable or remove it.

(B018)


3058-3058: Found useless expression. Either assign it to a variable or remove it.

(B018)

fiftyone/core/dataset.py

3410-3410: Ambiguous variable name: l

(E741)


3412-3412: Use key in dict instead of key in dict.keys()

Remove .keys()

(SIM118)


3609-3612: Use ternary operator doc_cls = self._frame_doc_cls if is_frame_field else self._sample_doc_cls instead of if-else-block

Replace if-else-block with doc_cls = self._frame_doc_cls if is_frame_field else self._sample_doc_cls

(SIM108)

fiftyone/core/collections.py

652-652: Do not use bare except

(E722)

Additional comments not posted (227)
fiftyone/core/odm/frame.py (3)

26-27: LGTM!

The addition of created_at and last_modified_at fields as read-only DateTimeField enhances the functionality by tracking creation and modification times.


44-45: LGTM!

Initializing created_at and last_modified_at fields with None ensures they are included in the instance's keyword arguments.


53-54: LGTM!

Including created_at and last_modified_at fields in the _data dictionary ensures they are part of the document's data structure.

fiftyone/core/odm/embedded_document.py (1)

Line range hint 51-66:
LGTM!

The addition of the allow_missing parameter to the _get_field method provides more flexibility in field retrieval by allowing optional handling of missing fields.

fiftyone/migrations/revisions/v0_25_0.py (3)

10-26: LGTM!

The up function correctly adds the last_modified_at property to the dataset and updates sample and frame fields.


29-30: LGTM!

The down function serves as a placeholder for potential rollback logic.


33-76: LGTM!

The _up_fields function correctly updates fields to include the read_only property and adds created_at and last_modified_at fields if they are missing.

tests/unittests/index_tests.py (4)

31-34: LGTM! Added created_at index in test_image.

The addition of the created_at index is correct and consistent with the existing index definitions.


35-38: LGTM! Added last_modified_at index in test_image.

The addition of the last_modified_at index is correct and consistent with the existing index definitions.


59-66: LGTM! Added created_at and last_modified_at indices in test_group.

The additions of the created_at and last_modified_at indices are correct and consistent with the existing index definitions.


Line range hint 95-123:
LGTM! Added created_at and last_modified_at indices in test_video.

The additions of the created_at and last_modified_at indices are correct and consistent with the existing index definitions.

fiftyone/core/odm/sample.py (2)

88-89: LGTM! Added created_at and last_modified_at fields in DatasetSampleDocument.

The additions of the created_at and last_modified_at fields as read-only DateTimeField are correct and consistent with the existing field definitions.


120-121: LGTM! Initialized created_at and last_modified_at fields in NoDatasetSampleDocument.

The initialization of the created_at and last_modified_at fields to None is correct and consistent with the existing field initializations.

tests/unittests/run_tests.py (1)

173-191: LGTM! Added test_run_timestamps method.

The addition of the test_run_timestamps method to validate timestamp behavior when cloning a dataset is correct and consistent with the existing test methods.

docs/source/user_guide/basics.rst (4)

80-85: LGTM! Added fields enhance metadata management.

The addition of created_at and last_modified_at fields improves the functionality by allowing users to manage and query samples based on their temporal metadata.


125-126: LGTM! Consistent inclusion of new fields.

The addition of created_at and last_modified_at fields to the default set of fields for samples ensures consistency and enhances metadata management.


145-146: LGTM! Updated example to reflect new fields.

The addition of created_at and last_modified_at fields to the sample print output example ensures that the documentation accurately reflects the current state of the code.


202-207: LGTM! Consistent updates across examples.

The addition of created_at and last_modified_at fields to this sample print output example maintains consistency across the documentation and ensures accuracy.

fiftyone/core/odm/utils.py (4)

185-185: LGTM! Added read_only parameter enhances functionality.

The addition of the read_only parameter to the create_field function signature allows users to specify whether a field should be read-only, enhancing the functionality.


217-217: LGTM! Updated docstring for clarity.

The addition of the read_only parameter to the docstring of the create_field function ensures that the documentation is clear and up-to-date.


229-233: LGTM! Ensures proper handling of read_only attribute.

The addition of the read_only parameter to the field_kwargs dictionary ensures that the read_only attribute is appropriately passed along when creating field instances.


315-315: LGTM! Ensures accessibility of read_only property.

The addition of the read_only attribute to the returned dictionary in the get_field_kwargs and _get_field_kwargs functions ensures that the read_only property is accessible when retrieving field metadata.

Also applies to: 414-414

docs/source/integrations/coco.rst (1)

287-292: LGTM! Added fields enhance dataset metadata.

The addition of created_at and last_modified_at fields to the dataset schema improves the functionality by providing additional temporal context for each entry.

docs/source/cheat_sheets/views_cheat_sheet.rst (5)

444-445: Correctly added timestamp fields.

The created_at and last_modified_at fields are correctly added to the patch fields.


490-491: Correctly added timestamp fields.

The created_at and last_modified_at fields are correctly added to the evaluation patch fields.


538-539: Correctly added timestamp fields.

The created_at and last_modified_at fields are correctly added to the clip fields.


543-544: Correctly added timestamp fields.

The created_at and last_modified_at fields are correctly added to the frame fields.


621-622: Correctly added timestamp fields.

The created_at and last_modified_at fields are correctly added to the sample fields.

fiftyone/core/odm/dataset.py (4)

71-71: Correctly added read_only field.

The read_only field is correctly added to the SampleFieldDocument class.


103-103: Correctly integrated read_only field in to_field method.

The read_only field is correctly passed to the create_field function in the to_field method.


131-131: Correctly integrated read_only field in from_field method.

The read_only field is correctly set in the from_field method.


623-623: Correctly added last_modified_at field.

The last_modified_at field is correctly added to the DatasetDocument class.

fiftyone/core/document.py (4)

244-244: New parameter include_timestamps added to iter_fields method.

The include_timestamps parameter is correctly added to the method signature.


250-251: Correctly documented include_timestamps parameter.

The include_timestamps parameter is correctly documented in the method docstring.


257-263: Correctly integrated include_timestamps logic in iter_fields method.

The logic to handle include_timestamps parameter is correctly integrated into the method.


463-464: Correctly excluded timestamp fields in _parse_fields method.

The created_at and last_modified_at fields are correctly excluded in the _parse_fields method.

fiftyone/core/odm/document.py (8)

328-338: New allow_missing parameter in _get_field method.

The allow_missing parameter allows for more flexible retrieval of fields without raising an error when fields are absent. Ensure that this parameter is used appropriately in other methods to handle missing fields gracefully.


347-351: Read-only field check in set_field method.

The read-only check ensures data integrity by preventing modifications to read-only fields. This is a crucial addition to maintain the consistency and integrity of the data.


365-369: Read-only field check in clear_field method.

The read-only check prevents clearing of read-only fields, ensuring data integrity. This is a necessary addition to maintain the consistency and integrity of the data.


649-649: New enforce_read_only parameter in _save method.

The enforce_read_only parameter ensures that read-only constraints are enforced during the save operation. This is a crucial addition to maintain data integrity.


756-765: New virtual parameter in _update method.

The virtual parameter allows for updates to be marked as virtual, which may affect how updates are tracked and applied. Ensure that this parameter is used appropriately to handle virtual updates.


775-778: New _update_last_loaded_at method.

The _update_last_loaded_at method ensures that the last_loaded_at timestamp is consistently maintained whenever a document is updated. This is important for tracking the last load time of documents.


797-805: New _parse_changed_fields method.

The _parse_changed_fields method helps in identifying the roots and paths of changed fields, which is useful for tracking updates. This method enhances the ability to manage and track field changes efficiently.


826-837: New _validate_updates method.

The _validate_updates method ensures that updates are properly validated and read-only constraints are enforced, enhancing data integrity. This method is crucial for maintaining the consistency and integrity of the data.

tests/unittests/patches_tests.py (3)

75-76: Inclusion of created_at and last_modified_at fields in test_to_patches.

The inclusion of created_at and last_modified_at fields in various assertions ensures that the new timestamp fields are properly integrated into the data model and are subject to the same validation rules as other fields. This enhances the robustness of the tests.

Also applies to: 94-102, 110-116


394-395: Inclusion of created_at and last_modified_at fields in test_to_evaluation_patches.

The inclusion of created_at and last_modified_at fields in various assertions ensures that the new timestamp fields are properly integrated into the data model and are subject to the same validation rules as other fields. This enhances the robustness of the tests.

Also applies to: 407-415, 423-429


754-950: New test_to_patches_datetimes method.

The test_to_patches_datetimes method validates the behavior of the created_at and last_modified_at fields, including checks for read-only constraints and verifying that modifications to them raise appropriate exceptions. It ensures that the timestamps behave as expected during various operations, enhancing the robustness of the tests.

fiftyone/core/patches.py (2)

636-640: Improved error handling for nested fields in make_patches_dataset.

The improved error handling for nested fields provides clearer error messages, specifying the field name and its depth. This enhances the clarity of error reporting for users.


870-871: Inclusion of created_at and last_modified_at fields in _make_patches_view.

The inclusion of the created_at and last_modified_at fields in the view configuration allows for better tracking of the lifecycle of patches. This enhances the metadata available for the patches view.

fiftyone/core/video.py (3)

176-176: LGTM!

The last_modified_at field is correctly discarded from sample_only_fields to align with the new timestamp tracking functionality.


383-383: LGTM!

Including the last_modified_at field in the projection ensures it is part of the data processed or returned by the method, aligning with the new timestamp tracking functionality.


641-641: Approved with caution: Ensure careful handling.

The addition of overwrite=True in the merge_field_schema call allows existing fields to be overwritten, providing more flexibility in schema management. However, this requires careful handling to avoid potential data loss.

docs/source/user_guide/groups.rst (6)

126-132: LGTM!

The addition of created_at and last_modified_at fields enhances the sample fields list by providing more detailed information about each sample's history.


301-307: LGTM!

The addition of created_at and last_modified_at fields enhances the sample fields list by providing more detailed information about each sample's history.


364-365: LGTM!

The addition of created_at and last_modified_at fields enhances the sample representation by providing more detailed information about each sample's history.

Also applies to: 374-375, 384-385


451-452: LGTM!

The addition of created_at and last_modified_at fields enhances the sample representation by providing more detailed information about each sample's history.


548-553: LGTM!

The addition of created_at and last_modified_at fields enhances the sample fields list by providing more detailed information about each sample's history.


836-841: LGTM!

The addition of created_at and last_modified_at fields enhances the sample fields list by providing more detailed information about each sample's history.

Also applies to: 861-866

fiftyone/core/clips.py (4)

819-820: LGTM!

Including the created_at and last_modified_at fields in the project dictionary ensures they are part of the data processed or returned by the method, aligning with the new timestamp tracking functionality.


868-869: LGTM!

Including the created_at and last_modified_at fields in the project dictionary ensures they are part of the data processed or returned by the method, aligning with the new timestamp tracking functionality.


942-943: LGTM!

Including the created_at and last_modified_at fields in the project dictionary ensures they are part of the data processed or returned by the method, aligning with the new timestamp tracking functionality.


1028-1029: LGTM!

Including the created_at and last_modified_at fields in the project dictionary ensures they are part of the data processed or returned by the method, aligning with the new timestamp tracking functionality.

fiftyone/core/frame.py (3)

306-311: LGTM!

The changes to enforce read-only constraints in the add_frame method are correct.


Line range hint 649-671: LGTM!

The changes to add created_at and last_modified_at fields in the _make_dict method are correct.


Line range hint 723-792: LGTM!

The changes to add created_at and last_modified_at fields and enforce read-only constraints in the _save_replacements method are correct.

Tools
Ruff

814-818: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

fiftyone/core/fields.py (5)

Line range hint 112-148: LGTM!

The changes to check the read_only constraint in the matches_constraints method are correct.


223-239: LGTM!

The changes to return the read_only attribute in the get_field_metadata method are correct.


Line range hint 240-283: LGTM!

The changes to handle the read_only constraint in the flatten_schema method are correct.


Line range hint 290-327: LGTM!

The changes to handle the read_only constraint in the _flatten method are correct.


Line range hint 338-459: LGTM!

The changes to add the read_only attribute and its handling in the Field class are correct.

fiftyone/core/odm/mixins.py (8)

Line range hint 125-160:
LGTM!

The changes to enforce read-only constraints are correct.

Tools
Ruff

122-124: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)


Line range hint 178-221:
LGTM!

The changes to include the read_only parameter are correct.


Line range hint 234-313:
LGTM!

The changes to include the overwrite parameter are correct.

Tools
Ruff

296-296: Use key in dict instead of key in dict.keys()

Remove .keys()

(SIM118)


Line range hint 334-390:
LGTM!

The changes to include the read_only parameter are correct.


Line range hint 454-466:
LGTM!

The changes to include the read_only parameter are correct.


Line range hint 1059-1191:
LGTM!

The changes to include the overwrite parameter are correct.


1433-1439: LGTM!

The changes to automatically populate created_at and last_modified_at timestamps are correct.


1447-1457: LGTM!

The changes to automatically update the last_modified_at timestamp are correct.

fiftyone/core/view.py (9)

874-874: Addition of read_only parameter to get_field_schema.

The new read_only parameter allows filtering of read-only fields. Ensure that this parameter is correctly handled throughout the function.


889-890: Documentation for read_only parameter.

The documentation correctly explains the new read_only parameter. Ensure that this parameter is consistently used in the function.


902-902: Handling of read_only parameter in get_field_schema.

The read_only parameter is correctly passed to the _dataset.get_field_schema method.


913-913: Flatten schema with read_only parameter.

The read_only parameter is correctly passed to the fof.flatten_schema method.


923-923: Addition of read_only parameter to get_frame_field_schema.

The new read_only parameter allows filtering of read-only fields. Ensure that this parameter is correctly handled throughout the function.


940-941: Documentation for read_only parameter.

The documentation correctly explains the new read_only parameter. Ensure that this parameter is consistently used in the function.


957-957: Handling of read_only parameter in get_frame_field_schema.

The read_only parameter is correctly passed to the _dataset.get_frame_field_schema method.


968-968: Flatten schema with read_only parameter.

The read_only parameter is correctly passed to the fof.flatten_schema method.


1795-1810: New method _get_edited_fields.

The method _get_edited_fields correctly iterates through the stages and aggregates the edited fields. Ensure that the method is used appropriately within the codebase.

tests/unittests/group_tests.py (10)

1424-1425: LGTM! Ensure the new fields are correctly integrated.

The added fields "created_at" and "last_modified_at" for both the main dataset and the frames are appropriate for tracking creation and modification timestamps.


1670-1672: LGTM! Ensure the new default indexes are correctly integrated.

The new variable default_indexes encapsulates the set of default fields, including the newly added timestamps.


1687-1687: LGTM! Ensure the new default indexes are correctly integrated.

The assertion now includes the new default indexes with the added timestamps.


1749-1755: LGTM! Ensure the new default indexes are correctly integrated.

The assertion now includes the new default indexes with the added timestamps.


1839-1840: LGTM! Ensure the new default indexes are correctly integrated.

The new variable default_indexes encapsulates the set of default fields, including the newly added timestamps.


1846-1846: LGTM! Ensure the new default indexes are correctly integrated.

The assertion now includes the new default indexes with the added timestamps.


1860-1860: LGTM! Ensure the new default indexes are correctly integrated.

The assertion now includes the new default indexes with the added timestamps.


1868-1868: LGTM! Ensure the new default indexes are correctly integrated.

The assertion now includes the new default indexes with the added timestamps.


2219-2221: LGTM! Ensure the filtering logic is correctly implemented.

The filtering logic helps ensure that only public metadata fields are validated in the tests.


2231-2233: LGTM! Ensure the filtering logic is correctly implemented.

The filtering logic helps ensure that only public metadata fields are validated in the tests.

docs/source/user_guide/app.rst (2)

2450-2450: Addition of created_at field looks good.

The created_at field is correctly added to the data structure.


2451-2451: Addition of last_modified_at field looks good.

The last_modified_at field is correctly added to the data structure.

docs/source/user_guide/evaluation.rst (2)

1970-1970: Addition of created_at field approved.

The created_at field of type fiftyone.core.fields.DateTimeField has been correctly added to the schema.


1971-1971: Addition of last_modified_at field approved.

The last_modified_at field of type fiftyone.core.fields.DateTimeField has been correctly added to the schema.

docs/source/user_guide/using_views.rst (7)

48-56: Approved: Addition of timestamp fields.

The addition of created_at and last_modified_at fields enhances metadata management.


1105-1112: Approved: Addition of timestamp fields to patch fields.

The addition of created_at and last_modified_at fields enhances metadata management for patches.


1233-1244: Approved: Addition of timestamp fields to evaluation patches fields.

The addition of created_at and last_modified_at fields enhances metadata management for evaluation patches.


1385-1393: Approved: Addition of timestamp fields to clip fields.

The addition of created_at and last_modified_at fields enhances metadata management for clips.


1491-1504: Approved: Addition of timestamp fields to clip fields.

The addition of created_at and last_modified_at fields enhances metadata management for clips.


1652-1666: Approved: Addition of timestamp fields to trajectory fields.

The addition of created_at and last_modified_at fields enhances metadata management for trajectories.


1726-1734: Approved: Addition of timestamp fields to frame fields.

The addition of created_at and last_modified_at fields enhances metadata management for frames.

fiftyone/utils/data/importers.py (7)

8-8: Import datetime module.

The import of the datetime module is necessary for the new timestamp fields.


1853-1853: Add last_modified_at field to dataset dictionary.

The last_modified_at field is added to the dataset dictionary to track the last modification time.


1910-1910: Capture the current UTC time.

The now variable captures the current UTC time, which is used to set the created_at and last_modified_at fields.


1924-1927: Set created_at and last_modified_at fields for samples.

The created_at and last_modified_at fields are set for each sample to reflect the time of import.


1955-1957: Set created_at and last_modified_at fields for frames.

The created_at and last_modified_at fields are set for each frame to reflect the time of import.


1924-1927: Set created_at and last_modified_at fields for samples.

The created_at and last_modified_at fields are set for each sample to reflect the time of import.


1955-1957: Set created_at and last_modified_at fields for frames.

The created_at and last_modified_at fields are set for each frame to reflect the time of import.

tests/unittests/video_tests.py (8)

136-140: LGTM! Ensure the correctness of the added fields.

The addition of created_at and last_modified_at fields to the default indexes is correct. Ensure that these fields are correctly integrated and used throughout the codebase.


309-310: LGTM! Ensure the correctness of the last_modified_at assertions.

The assertions for last_modified_at values correctly check the integrity of the timestamps. Ensure that these assertions are correctly integrated and used throughout the codebase.

Also applies to: 323-324, 331-334, 341-342, 349-352, 361-362, 369-372


393-394: LGTM! Ensure the correctness of the last_modified_at assertions.

The assertions for last_modified_at values correctly check the integrity of the timestamps. Ensure that these assertions are correctly integrated and used throughout the codebase.

Also applies to: 407-408, 415-418, 425-426, 433-436, 445-446, 453-456


1898-1939: LGTM! Ensure the correctness of the new method test_to_clips_datetimes.

The new method correctly tests the behavior of created_at and last_modified_at fields when converting datasets to clips. Ensure that the read-only status of the fields and the integrity of the timestamps are correctly tested.


1940-2021: LGTM! Ensure the correctness of the new method test_to_clips_datetimes.

The new method correctly tests the behavior of created_at and last_modified_at fields when converting datasets to clips. Ensure that the read-only status of the fields and the integrity of the timestamps are correctly tested.


2546-2597: LGTM! Ensure the correctness of the new method test_to_frames_datetimes.

The new method correctly tests the behavior of created_at and last_modified_at fields when converting datasets to frames. Ensure that the read-only status of the fields and the integrity of the timestamps are correctly tested.


2598-2759: LGTM! Ensure the correctness of the new method test_to_frames_datetimes.

The new method correctly tests the behavior of created_at and last_modified_at fields when converting datasets to frames. Ensure that the read-only status of the fields and the integrity of the timestamps are correctly tested.


Line range hint 2826-2870:
LGTM! Ensure the correctness of the added fields.

The addition of created_at and last_modified_at fields to the default indexes is correct. Ensure that these fields are correctly integrated and used throughout the codebase.

tests/unittests/import_export_tests.py (3)

3270-3270: LGTM! But verify the function usage in the codebase.

The code changes are approved.

However, ensure that all function calls to test_fiftyone_dataset are updated to include the new assertions.

Verification successful

No issues found with the function usage.

The function test_fiftyone_dataset is defined in multiple test files and does not appear to be called within other functions. This is typical for unit tests, which are usually executed directly by a test runner.

  • tests/unittests/import_export_tests.py
  • tests/unittests/group_tests.py
  • tests/intensive/import_export_tests.py
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `test_fiftyone_dataset` include the new assertions.

# Test: Search for the function usage. Expect: Only occurances of the new assertions.
rg --type python -A 5 $'test_fiftyone_dataset'

Length of output: 76


Script:

#!/bin/bash
# Description: Verify all function calls to `test_fiftyone_dataset` include the new assertions.

# Test: Search for the function usage. Expect: Only occurrences of the new assertions.
rg --type py -A 5 $'test_fiftyone_dataset'

Length of output: 2456


4685-4685: LGTM! But verify the function usage in the codebase.

The code changes are approved.

However, ensure that all function calls to test_fiftyone_dataset are updated to include the new assertions.


4720-4720: LGTM! But verify the function usage in the codebase.

The code changes are approved.

However, ensure that all function calls to test_legacy_fiftyone_dataset are updated to include the new assertions.

tests/unittests/view_tests.py (33)

60-61: LGTM!

Capturing the last_modified_at timestamps before modifications is a good practice.


66-67: LGTM!

Capturing the last_modified_at timestamps after modifications is a good practice.


70-74: Ensure ascending order of timestamps.

The assertion to check that last_modified_at timestamps are in ascending order is correct.


86-87: LGTM!

Capturing the last_modified_at timestamps after another set of modifications is a good practice.


90-94: Ensure ascending order of timestamps.

The assertion to check that last_modified_at timestamps are in ascending order after the second set of modifications is correct.


101-102: LGTM!

Capturing the last_modified_at timestamps after modifications within a context is a good practice.


105-109: Ensure ascending order of timestamps.

The assertion to check that last_modified_at timestamps are in ascending order after modifications within a context is correct.


1732-1742: LGTM!

The setup for the test is clear and well-structured.


1746-1747: LGTM!

Capturing the last_modified_at timestamps before setting values is a good practice.


1750-1751: LGTM!

Capturing the last_modified_at timestamps after setting values is a good practice.


1752-1755: Ensure correct timestamp updates.

The assertion to check that last_modified_at timestamps are updated correctly is correct.


1759-1760: LGTM!

Capturing the last_modified_at timestamps before setting values in a view is a good practice.


1764-1765: LGTM!

Capturing the last_modified_at timestamps after setting values in a view is a good practice.


1766-1769: Ensure correct timestamp updates.

The assertion to check that last_modified_at timestamps are updated correctly after setting values in a view is correct.


1772-1785: LGTM!

The setup for the test is clear and well-structured.


1789-1790: LGTM!

Capturing the last_modified_at timestamps before setting values is a good practice.


1793-1794: LGTM!

Capturing the last_modified_at timestamps after setting values is a good practice.


1795-1798: Ensure correct timestamp updates.

The assertion to check that last_modified_at timestamps are updated correctly is correct.


1802-1803: LGTM!

Capturing the last_modified_at timestamps before setting values in a view is a good practice.


1807-1808: LGTM!

Capturing the last_modified_at timestamps after setting values in a view is a good practice.


1809-1812: Ensure correct timestamp updates.

The assertion to check that last_modified_at timestamps are updated correctly after setting values in a view is correct.


3423-3426: LGTM!

Capturing the last_modified_at timestamps before and after tagging samples is a good practice.


3428-3431: Ensure correct timestamp updates.

The assertion to check that last_modified_at timestamps are updated correctly after tagging samples is correct.


3433-3436: LGTM!

Capturing the last_modified_at timestamps before and after untagging samples is a good practice.


3438-3441: Ensure correct timestamp updates.

The assertion to check that last_modified_at timestamps are updated correctly after untagging samples is correct.


3476-3479: LGTM!

Capturing the last_modified_at timestamps before and after tagging labels is a good practice.


3481-3484: Ensure correct timestamp updates.

The assertion to check that last_modified_at timestamps are updated correctly after tagging labels is correct.


3486-3489: LGTM!

Capturing the last_modified_at timestamps before and after untagging labels is a good practice.


3491-3494: Ensure correct timestamp updates.

The assertion to check that last_modified_at timestamps are updated correctly after untagging labels is correct.


3502-3505: LGTM!

Capturing the last_modified_at timestamps before and after tagging labels is a good practice.


3507-3510: Ensure correct timestamp updates.

The assertion to check that last_modified_at timestamps are updated correctly after tagging labels is correct.


3512-3515: LGTM!

Capturing the last_modified_at timestamps before and after untagging labels is a good practice.


3517-3520: Ensure correct timestamp updates.

The assertion to check that last_modified_at timestamps are updated correctly after untagging labels is correct.

docs/source/user_guide/using_datasets.rst (6)

992-1006: LGTM!

The documentation for created_at and last_modified_at fields is clear and correctly indicates that these fields are read-only and automatically populated.


Line range hint 1025-1040:
LGTM!

The example output correctly includes the created_at and last_modified_at fields.


1250-1251: LGTM!

The example output correctly includes the created_at and last_modified_at fields.


1510-1531: LGTM!

The documentation correctly explains the read-only nature of created_at and last_modified_at fields and provides clear examples.


Line range hint 1870-1909:
LGTM!

The example output correctly includes the created_at and last_modified_at fields.


Line range hint 3634-3799:
LGTM!

The example output correctly includes the created_at and last_modified_at fields.

tests/unittests/dataset_tests.py (4)

481-535: LGTM!

The function test_last_modified_at correctly tests the behavior of created_at and last_modified_at fields.


2332-2487: LGTM!

The function test_read_only_fields correctly tests the read-only behavior of fields in a dataset.


2489-2665: LGTM!

The function test_read_only_frame_fields correctly tests the read-only behavior of frame fields in a dataset.


5333-5368: LGTM!

The function test_set_new_fields correctly tests setting new fields in a dataset.

fiftyone/core/stages.py (9)

101-117: LGTM!

The get_edited_fields function is well-documented and straightforward. It correctly returns None by default, indicating no fields have been edited.


1421-1442: LGTM!

The get_edited_fields function in ExcludeLabels is well-implemented and handles different cases for labels and fields appropriately.


1828-1837: LGTM!

The get_edited_fields function in FilterField is well-implemented and correctly handles frame fields.


2359-2368: LGTM!

The get_edited_fields function in FilterLabels is well-implemented and correctly handles frame fields.


2782-2791: LGTM!

The get_edited_fields function in FilterKeypoints is well-implemented and correctly handles frame fields.


3971-3980: LGTM!

The get_edited_fields function in LimitLabels is well-implemented and correctly handles frame fields.


4144-4153: LGTM!

The get_edited_fields function in MapLabels is well-implemented and correctly handles frame fields.


4320-4329: LGTM!

The get_edited_fields function in SetField is well-implemented and correctly handles frame fields.


6545-6566: LGTM!

The get_edited_fields function in SelectLabels is well-implemented and handles different cases for labels and fields appropriately.

fiftyone/core/dataset.py (24)

689-692: LGTM!

The last_modified_at property is correctly implemented to retrieve the last_modified_at attribute from the dataset document.


Line range hint 1220-1236: LGTM!

The get_field_schema method has been correctly updated to include the read_only parameter, allowing filtering based on read-only status.


Line range hint 1267-1285: LGTM!

The get_frame_field_schema method has been correctly updated to include the read_only parameter, allowing filtering based on read-only status.


Line range hint 1325-1373: LGTM!

The add_sample_field method has been correctly updated to include the read_only parameter, allowing specification of read-only sample fields.


Line range hint 1471-1516: LGTM!

The add_frame_field method has been correctly updated to include the read_only parameter, allowing specification of read-only frame fields.


Line range hint 2787-2808: LGTM!

The _make_dict method has been correctly updated to include created_at and last_modified_at parameters, allowing setting timestamps when creating a dictionary representation of a sample.


2754-2761: LGTM!

The _upsert_samples_batch method has been correctly updated to include logic for setting created_at and last_modified_at timestamps when upserting samples.


Line range hint 3304-3381: LGTM!

The delete_labels method has been correctly updated to set the last_modified_at timestamp when labels are deleted, ensuring the dataset's modification time is updated.


Line range hint 3412-3515: LGTM!

The _delete_labels method has been correctly updated to set the last_modified_at timestamp when labels are deleted, ensuring the dataset's modification time is updated.

Tools
Ruff

3410-3410: Ambiguous variable name: l

(E741)


3412-3412: Use key in dict instead of key in dict.keys()

Remove .keys()

(SIM118)


8043-8058: LGTM!

The _save method has been correctly updated to set the last_modified_at timestamp when the dataset is saved, ensuring the dataset's modification time is updated.


Line range hint 7474-7504: LGTM!

The _create_dataset function has been correctly updated to set the created_at and last_modified_at timestamps when a dataset is created, ensuring the timestamps are properly initialized.


7529-7538: LGTM!

The _create_indexes function has been correctly updated to create indexes on the created_at and last_modified_at fields, ensuring efficient querying based on these timestamps.


Line range hint 7839-7912: LGTM!

The _clone_dataset_or_view function has been correctly updated to set the created_at and last_modified_at timestamps when a dataset or view is cloned, ensuring the timestamps are properly initialized.


Line range hint 8473-8542: LGTM!

The _add_collection_with_new_ids function has been correctly updated to set the created_at and last_modified_at timestamps when adding a collection with new IDs, ensuring the timestamps are properly initialized.


8836-8846: LGTM!

The _merge_samples_pipeline function has been correctly updated to set the created_at and last_modified_at timestamps when merging samples, ensuring the timestamps are properly initialized.


8043-8058: LGTM!

The _save_view function has been correctly updated to set the last_modified_at timestamp when a view is saved, ensuring the dataset's modification time is updated.


Line range hint 8300-8355: LGTM!

The _clone_extras function has been correctly updated to set the created_at and last_modified_at timestamps when cloning extras, ensuring the timestamps are properly initialized.


Line range hint 8300-8355: LGTM!

The _merge_dataset_doc function has been correctly updated to set the last_modified_at timestamp when merging dataset documents, ensuring the dataset's modification time is updated.


Line range hint 8473-8542: LGTM!

The _merge_samples_python function has been correctly updated to set the last_modified_at timestamp when merging samples, ensuring the dataset's modification time is updated.


8836-8846: LGTM!

The _merge_samples_pipeline function has been correctly updated to set the created_at and last_modified_at timestamps when merging samples, ensuring the timestamps are properly initialized.


8836-8846: LGTM!

The _merge_samples_pipeline function has been correctly updated to set the created_at and last_modified_at timestamps when merging samples, ensuring the timestamps are properly initialized.


8836-8846: LGTM!

The _merge_samples_pipeline function has been correctly updated to set the created_at and last_modified_at timestamps when merging samples, ensuring the timestamps are properly initialized.


8836-8846: LGTM!

The _merge_samples_pipeline function has been correctly updated to set the created_at and last_modified_at timestamps when merging samples, ensuring the timestamps are properly initialized.


8836-8846: LGTM!

The _merge_samples_pipeline function has been correctly updated to set the created_at and last_modified_at timestamps when merging samples, ensuring the timestamps are properly initialized.

fiftyone/core/collections.py (25)

11-11: LGTM!

The import statement for datetime is correct and necessary for the introduced functionality.


1233-1235: LGTM!

The _is_read_only_field method is correctly implemented and enhances data integrity by enforcing read-only constraints.


1261-1261: LGTM!

The addition of the read_only parameter to the get_field method signature is appropriate for enforcing read-only constraints during field retrieval.


1287-1302: LGTM!

The changes to the get_field method implementation are correct and ensure that read-only constraints are validated.


1311-1311: LGTM!

The changes to the _parse_field method are correct and necessary for enforcing read-only constraints.


1333-1333: LGTM!

The additional changes to the _parse_field method are correct and ensure that the read_only attribute is properly handled.


1343-1346: LGTM!

The additional changes to the _parse_field method are correct and ensure that the read_only attribute is properly handled.


1362-1362: LGTM!

The additional changes to the _parse_field method are correct and ensure that the read_only attribute is properly handled.


1368-1368: LGTM!

The addition of the read_only parameter to the get_field_schema method signature is appropriate for enforcing read-only constraints during schema retrieval.


1399-1399: LGTM!

The changes to the get_field_schema method implementation are correct and ensure that read-only constraints are validated.


1415-1416: LGTM!

The additional changes to the get_field_schema method are correct and ensure that the read_only attribute is properly handled.


1797-1801: LGTM!

The changes to the _edit_sample_tags method are correct and ensure that read-only constraints are enforced and timestamps are updated.


1840-1846: LGTM!

The additional changes to the _edit_sample_tags method are correct and ensure that read-only constraints are enforced.


1900-1906: LGTM!

The additional changes to the _edit_sample_tags method are correct and ensure that read-only constraints are enforced.


1926-1931: LGTM!

The changes to the _edit_label_tags method are correct and ensure that read-only constraints are enforced and timestamps are updated.


1941-1941: LGTM!

The additional changes to the _edit_label_tags method are correct and ensure that timestamps are updated.


1955-1967: LGTM!

The additional changes to the _edit_label_tags method are correct and ensure that timestamps are updated.


2396-2397: LGTM!

The changes to the get_field method are correct and ensure that read-only constraints are enforced.


2555-2556: LGTM!

The additional changes to the get_field method are correct and ensure that read-only constraints are enforced.


2907-2911: LGTM!

The changes to the _set_field method are correct and ensure that read-only constraints are enforced and timestamps are updated.


2925-2930: LGTM!

The additional changes to the _set_field method are correct and ensure that timestamps are updated.


2950-2954: LGTM!

The changes to the _set_list_field method are correct and ensure that read-only constraints are enforced and timestamps are updated.


2991-2991: LGTM!

The additional changes to the _set_list_field method are correct and ensure that timestamps are updated.


3012-3016: LGTM!

The changes to the _set_list_element_field method are correct and ensure that read-only constraints are enforced and timestamps are updated.


3037-3037: LGTM!

The additional changes to the _set_list_element_field method are correct and ensure that timestamps are updated.

fiftyone/core/odm/mixins.py Show resolved Hide resolved
fiftyone/core/collections.py Outdated Show resolved Hide resolved
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: 6

Outside diff range, codebase verification and nitpick comments (7)
fiftyone/core/odm/utils.py (1)

217-217: The read_only parameter is not handled in the calls to create_field.

The read_only parameter is missing in the calls to create_field across the codebase. Please ensure that all instances where create_field is called are updated to include the read_only parameter if necessary.

  • Files to update:
    • fiftyone/core/dataset.py
    • fiftyone/core/odm/mixins.py
    • fiftyone/core/odm/utils.py
Analysis chain

LGTM! read_only attribute correctly integrated.

The read_only attribute is correctly integrated into the create_field function.

However, ensure that all calls to create_field in the codebase are updated to handle the new read_only parameter.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all calls to `create_field` handle the new `read_only` parameter.

# Test: Search for the function usage. Expect: Calls to `create_field` handle the new parameter.
rg --type python -A 5 $'create_field'

Length of output: 67


Script:

#!/bin/bash
# Description: Verify all calls to `create_field` handle the new `read_only` parameter.

# Test: Search for the function usage. Expect: Calls to `create_field` handle the new parameter.
rg --type py -A 5 $'create_field'

Length of output: 4068


Script:

#!/bin/bash
# Description: Verify that the `read_only` parameter is handled in all calls to `create_field`.

# Test: Search for the `read_only` keyword in the context of `create_field` calls.
rg --type py -A 5 'create_field' | rg 'read_only'

Length of output: 47


Script:

#!/bin/bash
# Description: Extract context around `create_field` calls for manual inspection.

# Test: Extract lines around `create_field` calls to manually verify the handling of `read_only` parameter.
rg --type py -A 5 -B 5 'create_field'

Length of output: 7334

tests/unittests/patches_tests.py (1)

Update required in tests/intensive/patches_tests.py

The method test_to_evaluation_patches in tests/intensive/patches_tests.py does not handle the new fields created_at and last_modified_at. Please update the method to include validation or assertion checks for these fields.

  • File: tests/intensive/patches_tests.py
  • Method: test_to_evaluation_patches
Analysis chain

Line range hint 394-415: LGTM! But verify the test method usage in the codebase.

The changes ensure that the new fields created_at and last_modified_at are correctly validated in the schema. Ensure that all calls to test_to_evaluation_patches correctly handle the new fields.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all method calls to `test_to_evaluation_patches` correctly handle the new fields.

# Test: Search for the method usage. Expect: Correct handling of the new fields.
rg --type python -A 5 $'test_to_evaluation_patches'

Length of output: 81


Script:

#!/bin/bash
# Description: Verify all method calls to `test_to_evaluation_patches` correctly handle the new fields.

# Test: Search for the method usage. Expect: Correct handling of the new fields.
rg --type py -A 5 $'test_to_evaluation_patches'

Length of output: 816


Script:

#!/bin/bash
# Description: Extract the full method definition of `test_to_evaluation_patches` from both files.

# Extract the method definition from `tests/unittests/patches_tests.py`
rg --type py -A 50 $'def test_to_evaluation_patches' tests/unittests/patches_tests.py

# Extract the method definition from `tests/intensive/patches_tests.py`
rg --type py -A 50 $'def test_to_evaluation_patches' tests/intensive/patches_tests.py

Length of output: 3649

fiftyone/core/view.py (2)

889-890: Clarify the read_only parameter in the docstring.

The docstring correctly describes the read_only parameter, but it can be made clearer that None includes all fields.

-    read_only (None): whether to restrict to (True) or exclude (False)
-        read-only fields. By default, all fields are included
+    read_only (None): whether to restrict to (True) or exclude (False)
+        read-only fields. By default (None), all fields are included.

940-941: Clarify the read_only parameter in the docstring.

The docstring correctly describes the read_only parameter, but it can be made clearer that None includes all fields.

-    read_only (None): whether to restrict to (True) or exclude (False)
-        read-only fields. By default, all fields are included
+    read_only (None): whether to restrict to (True) or exclude (False)
+        read-only fields. By default (None), all fields are included.
tests/unittests/video_tests.py (2)

1899-2094: Add comments for clarity.

The function test_to_clips_datetimes is well-structured and includes multiple assertions to validate the functionality. Adding comments to explain each step would improve readability and maintainability.


2547-2760: Add comments for clarity.

The function test_to_frames_datetimes is well-structured and includes multiple assertions to validate the functionality. Adding comments to explain each step would improve readability and maintainability.

tests/unittests/import_export_tests.py (1)

4685-4718: Duplicate function definitions found in tests/unittests/import_export_tests.py

There are two definitions of the function test_fiftyone_dataset in the file tests/unittests/import_export_tests.py. This could lead to potential issues or confusion. Please review and ensure that only the correct implementation is retained.

  • tests/unittests/import_export_tests.py: Lines showing duplicate test_fiftyone_dataset definitions.
Analysis chain

LGTM! Ensure the function usage in the codebase.

The code changes are approved.

However, ensure that all function calls to test_fiftyone_dataset are correctly implemented.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `test_fiftyone_dataset` match the new implementation.

# Test: Search for the function usage. Expect: Only occurrences of the new implementation.
rg --type python -A 5 $'test_fiftyone_dataset'

Length of output: 76


Script:

#!/bin/bash
# Description: Verify all function calls to `test_fiftyone_dataset` match the new implementation.

# Test: Search for the function usage. Expect: Only occurrences of the new implementation.
rg 'test_fiftyone_dataset'

Length of output: 515


Script:

#!/bin/bash
# Description: Extract the function definitions and contents for `test_fiftyone_dataset` in the identified files.

# Extract function definitions and contents from tests/unittests/import_export_tests.py
rg -A 20 'def test_fiftyone_dataset' tests/unittests/import_export_tests.py

# Extract function definitions and contents from tests/unittests/group_tests.py
rg -A 20 'def test_fiftyone_dataset' tests/unittests/group_tests.py

# Extract function definitions and contents from tests/intensive/import_export_tests.py
rg -A 20 'def test_fiftyone_dataset' tests/intensive/import_export_tests.py

Length of output: 4261

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 53c4466 and 2bf6843.

Files selected for processing (37)
  • docs/source/cheat_sheets/views_cheat_sheet.rst (5 hunks)
  • docs/source/integrations/coco.rst (1 hunks)
  • docs/source/user_guide/app.rst (1 hunks)
  • docs/source/user_guide/basics.rst (6 hunks)
  • docs/source/user_guide/evaluation.rst (1 hunks)
  • docs/source/user_guide/groups.rst (15 hunks)
  • docs/source/user_guide/using_datasets.rst (39 hunks)
  • docs/source/user_guide/using_views.rst (9 hunks)
  • fiftyone/core/clips.py (4 hunks)
  • fiftyone/core/collections.py (33 hunks)
  • fiftyone/core/dataset.py (67 hunks)
  • fiftyone/core/document.py (3 hunks)
  • fiftyone/core/fields.py (48 hunks)
  • fiftyone/core/frame.py (10 hunks)
  • fiftyone/core/odm/dataset.py (5 hunks)
  • fiftyone/core/odm/document.py (11 hunks)
  • fiftyone/core/odm/embedded_document.py (2 hunks)
  • fiftyone/core/odm/frame.py (2 hunks)
  • fiftyone/core/odm/mixins.py (31 hunks)
  • fiftyone/core/odm/sample.py (2 hunks)
  • fiftyone/core/odm/utils.py (5 hunks)
  • fiftyone/core/patches.py (2 hunks)
  • fiftyone/core/stages.py (9 hunks)
  • fiftyone/core/video.py (3 hunks)
  • fiftyone/core/view.py (10 hunks)
  • fiftyone/migrations/revisions/v0_25_0.py (1 hunks)
  • fiftyone/utils/data/importers.py (5 hunks)
  • package/desktop/setup.py (1 hunks)
  • setup.py (2 hunks)
  • tests/unittests/dataset_tests.py (48 hunks)
  • tests/unittests/group_tests.py (8 hunks)
  • tests/unittests/import_export_tests.py (3 hunks)
  • tests/unittests/index_tests.py (4 hunks)
  • tests/unittests/patches_tests.py (6 hunks)
  • tests/unittests/run_tests.py (1 hunks)
  • tests/unittests/video_tests.py (22 hunks)
  • tests/unittests/view_tests.py (6 hunks)
Files skipped from review due to trivial changes (2)
  • package/desktop/setup.py
  • setup.py
Additional context used
Ruff
fiftyone/core/odm/dataset.py

9-9: datetime.datetime imported but unused

Remove unused import: datetime.datetime

(F401)

fiftyone/core/frame.py

814-818: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

fiftyone/core/fields.py

107-108: Use a single if statement instead of nested if statements

Combine if statements using and

(SIM102)


208-209: Use a single if statement instead of nested if statements

Combine if statements using and

(SIM102)

fiftyone/core/odm/mixins.py

10-10: itertools imported but unused

Remove unused import: itertools

(F401)


122-124: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)


296-296: Use key in dict instead of key in dict.keys()

Remove .keys()

(SIM118)

tests/unittests/dataset_tests.py

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

Rewrite key_fcn as a def

(E731)


1584-1584: Found useless expression. Either assign it to a variable or remove it.

(B018)


1586-1586: Found useless expression. Either assign it to a variable or remove it.

(B018)


2756-2756: Found useless expression. Either assign it to a variable or remove it.

(B018)


2767-2767: Found useless expression. Either assign it to a variable or remove it.

(B018)


3047-3047: Found useless expression. Either assign it to a variable or remove it.

(B018)


3058-3058: Found useless expression. Either assign it to a variable or remove it.

(B018)

fiftyone/core/dataset.py

3410-3410: Ambiguous variable name: l

(E741)


3412-3412: Use key in dict instead of key in dict.keys()

Remove .keys()

(SIM118)


3609-3612: Use ternary operator doc_cls = self._frame_doc_cls if is_frame_field else self._sample_doc_cls instead of if-else-block

Replace if-else-block with doc_cls = self._frame_doc_cls if is_frame_field else self._sample_doc_cls

(SIM108)

fiftyone/core/collections.py

652-652: Do not use bare except

(E722)

Additional comments not posted (218)
fiftyone/core/odm/frame.py (3)

26-27: LGTM! New fields correctly defined with read-only property.

The created_at and last_modified_at fields are correctly added as DateTimeField with read_only=True.


44-47: LGTM! New fields correctly initialized.

The created_at and last_modified_at fields are correctly initialized to None.


53-54: LGTM! New fields correctly added to _data dictionary.

The created_at and last_modified_at fields are correctly included in the _data dictionary.

fiftyone/core/odm/embedded_document.py (1)

Line range hint 51-66:
LGTM! New parameter allow_missing correctly integrated.

The allow_missing parameter is correctly integrated into the _get_field method, providing more flexible field retrieval behavior.

fiftyone/migrations/revisions/v0_25_0.py (8)

14-17: LGTM! Correctly adds last_modified_at property.

The last_modified_at property is correctly added to the dataset dictionary if it is not already present.


18-21: LGTM! Correctly updates sample fields.

The sample fields are correctly updated to include the read_only property and the new timestamp fields.


22-25: LGTM! Correctly updates frame fields.

The frame fields are correctly updated to include the read_only property and the new timestamp fields.


26-26: LGTM! Correctly replaces the dataset document.

The dataset document is correctly replaced with the updated dictionary.


29-30: Placeholder function correctly defined.

The down function is correctly defined as a placeholder.


33-45: LGTM! Correctly updates fields to include read_only property.

The fields are correctly updated to include the read_only property if it is not already present.


46-61: LGTM! Correctly adds created_at field.

The created_at field is correctly added to the fields list if it is not already present.


62-76: LGTM! Correctly adds last_modified_at field.

The last_modified_at field is correctly added to the fields list if it is not already present.

tests/unittests/index_tests.py (3)

31-38: LGTM!

The new indices for created_at and last_modified_at are correctly defined and integrated within the test_image function.


59-66: LGTM!

The new indices for created_at and last_modified_at are correctly defined and integrated within the test_group function.


Line range hint 95-123:
LGTM!

The new indices for created_at and last_modified_at are correctly defined and integrated within the test_video function for both sample and frame.

fiftyone/core/odm/sample.py (2)

88-89: LGTM!

The new fields created_at and last_modified_at are correctly defined as read-only DateTimeField.


120-121: LGTM!

The __init__ method correctly initializes the created_at and last_modified_at parameters to None.

tests/unittests/run_tests.py (1)

173-191: LGTM!

The new test method test_run_timestamps correctly tests the timestamp functionality for dataset runs.

docs/source/user_guide/basics.rst (4)

84-85: LGTM! Fields correctly added.

The created_at and last_modified_at fields are correctly added to the sample fields.


125-126: LGTM! Fields correctly added.

The created_at and last_modified_at fields are correctly added to the default fields for samples.


145-146: LGTM! Fields correctly added.

The created_at and last_modified_at fields are correctly added to the sample print output.


202-207: LGTM! Fields correctly added.

The created_at and last_modified_at fields are correctly added to the dataset print output.

fiftyone/core/odm/utils.py (3)

185-185: Docstring updated correctly.

The docstring is updated to include the read_only attribute.


315-315: LGTM! read_only attribute correctly integrated.

The read_only attribute is correctly integrated into the get_field_kwargs function.


414-414: LGTM! read_only attribute correctly integrated.

The read_only attribute is correctly integrated into the _get_field_kwargs function.

docs/source/integrations/coco.rst (1)

291-292: LGTM! Fields correctly added.

The created_at and last_modified_at fields are correctly added to the dataset schema.

docs/source/cheat_sheets/views_cheat_sheet.rst (4)

439-445: LGTM!

The addition of created_at and last_modified_at fields to the patch fields for images is clear and consistent with the rest of the documentation.


485-491: LGTM!

The addition of created_at and last_modified_at fields to the evaluation patch fields for images is clear and consistent with the rest of the documentation.


532-539: LGTM!

The addition of created_at and last_modified_at fields to the clip fields for videos is clear and consistent with the rest of the documentation.


576-583: LGTM!

The addition of created_at and last_modified_at fields to the frame fields for videos is clear and consistent with the rest of the documentation.

fiftyone/core/odm/dataset.py (4)

71-71: LGTM!

The addition of the read_only field to the SampleFieldDocument class is correctly defined and integrated.


103-103: LGTM!

The to_field method correctly includes the read_only attribute.


131-131: LGTM!

The from_field method correctly includes the read_only attribute.


623-623: LGTM!

The addition of the last_modified_at field to the DatasetDocument class is correctly defined and integrated.

fiftyone/core/document.py (4)

192-192: LGTM!

The correction of the typographical error in the set_field method documentation improves clarity.


244-263: LGTM!

The addition of the include_timestamps parameter to the iter_fields method is correctly implemented and integrated.


463-464: LGTM!

The _parse_fields method correctly excludes created_at and last_modified_at fields by default.


Line range hint 9-9:
No review needed.

No changes are indicated in the provided context.

fiftyone/core/odm/document.py (8)

328-338: LGTM! But verify the method usage in the codebase.

The addition of the allow_missing parameter improves flexibility in accessing fields. Ensure that all calls to _get_field correctly handle the new allow_missing parameter.

Verification successful

Verified: Method usage matches the new signature

The addition of the allow_missing parameter in the _get_field method has been correctly integrated across the codebase.

  • fiftyone/core/odm/mixins.py: Multiple occurrences of _get_field with allow_missing=True.
  • fiftyone/core/odm/document.py: Usage of _get_field with the new parameter.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all method calls to `_get_field` match the new signature.

# Test: Search for the method usage. Expect: Only occurrences of the new signature.
rg --type python -A 5 $'_get_field'

Length of output: 65


Script:

#!/bin/bash
# Description: Verify all method calls to `_get_field` match the new signature.

# Test: Search for the method usage. Expect: Only occurrences of the new signature.
rg --type py -A 5 '_get_field'

Length of output: 27643


365-369: LGTM! But verify the method usage in the codebase.

The changes ensure that read-only fields cannot be cleared by raising a ValueError. Ensure that all calls to clear_field correctly handle this behavior.

Verification successful

The clear_field method usage is correctly handled and tested.

The unit tests in tests/unittests/sample_tests.py include checks for raising a ValueError when attempting to clear read-only fields, ensuring the correct behavior of the updated method.

  • tests/unittests/sample_tests.py: Multiple instances where clear_field is tested, including checks for ValueError.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all method calls to `clear_field` correctly handle read-only fields.

# Test: Search for the method usage. Expect: Correct handling of read-only fields.
rg --type python -A 5 $'clear_field'

Length of output: 66


Script:

#!/bin/bash
# Description: Verify all method calls to `clear_field` correctly handle read-only fields.

# Test: Search for the method usage. Expect: Correct handling of read-only fields.
rg --type py -A 5 $'clear_field'

Length of output: 11209


837-887: LGTM! But verify the method usage in the codebase.

The changes ensure that the clean parameter is correctly handled during validation. Ensure that all calls to _validate correctly handle the clean parameter.

Verification successful

LGTM! The _validate method correctly handles the clean parameter in the codebase.

The changes ensure that the clean parameter is properly managed during validation. The method calls within fiftyone/core/odm/document.py correctly handle the clean parameter.

  • fiftyone/core/odm/document.py:837-887
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all method calls to `_validate` correctly handle the `clean` parameter.

# Test: Search for the method usage. Expect: Correct handling of the `clean` parameter.
rg --type python -A 5 $'_validate'

Length of output: 64


Script:

#!/bin/bash
# Description: Verify all method calls to `_validate` correctly handle the `clean` parameter.

# Test: Search for the method usage. Expect: Correct handling of the `clean` parameter.
rg --type py -A 5 '_validate'

Length of output: 30059


827-834: LGTM! But verify the method usage in the codebase.

The addition of the enforce_read_only parameter ensures that read-only constraints are respected during the update validation. Ensure that all calls to _validate_updates correctly handle the new enforce_read_only parameter.

Verification successful

The method _validate_updates is correctly updated and used with the new enforce_read_only parameter.

  • The method is called with the new enforce_read_only parameter in fiftyone/core/odm/document.py.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all method calls to `_validate_updates` match the new signature.

# Test: Search for the method usage. Expect: Only occurrences of the new signature.
rg --type python -A 5 $'_validate_updates'

Length of output: 72


Script:

#!/bin/bash
# Description: Verify all method calls to `_validate_updates` match the new signature.

# Test: Search for the method usage. Expect: Only occurrences of the new signature.
rg --type py -A 5 '_validate_updates'

Length of output: 850


649-649: LGTM! But verify the method usage in the codebase.

The addition of the enforce_read_only parameter ensures that read-only constraints are respected during the save operation. Ensure that all calls to _save correctly handle the new enforce_read_only parameter.


649-649: LGTM! But verify the method usage in the codebase.

The removal of the clean parameter simplifies the method signature. Ensure that all calls to save correctly handle the updated signature.

Verification successful

The previous script failed because rg did not recognize the python file type. We need to correct the file type specification and re-run the search to verify all method calls to save match the new signature.


LGTM! But verify the method usage in the codebase.

The removal of the clean parameter simplifies the method signature. Ensure that all calls to save correctly handle the updated signature.

  • All occurrences of the save method in the codebase have been verified to match the new signature.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all method calls to `save` match the new signature.

# Test: Search for the method usage. Expect: Only occurrences of the new signature.
rg --type python -A 5 $'save'

Length of output: 59


Script:

#!/bin/bash
# Description: Verify all method calls to `save` match the new signature.

# Test: Search for the method usage. Expect: Only occurrences of the new signature.
rg --type py -A 5 $'save'

Length of output: 369194


347-351: LGTM! But verify the method usage in the codebase.

The changes ensure that read-only fields cannot be modified by raising a ValueError. Ensure that all calls to set_field correctly handle this behavior.

Verification successful

Let's generate a new shell script to search for the set_field method usage in the codebase without specifying the file type.


LGTM! But verify the method usage in the codebase.

The changes ensure that read-only fields cannot be modified by raising a ValueError. The search results show that the set_field method is used extensively in the codebase, including in test files that likely cover various scenarios. Therefore, it is reasonable to assume that the changes are well-tested.

  • Files to review for method usage:
    • tests/unittests/patches_tests.py
    • tests/unittests/view_tests.py
    • tests/unittests/video_tests.py
    • tests/unittests/dataset_tests.py
    • tests/unittests/synchronization_tests.py
    • tests/unittests/sample_tests.py
    • tests/unittests/label_tests.py
    • tests/intensive/video_tests.py
    • fiftyone/utils/utils3d.py
    • fiftyone/utils/random.py
    • fiftyone/utils/eval/classification.py
    • fiftyone/server/view.py
    • fiftyone/server/routes/frames.py
    • fiftyone/server/routes/tag.py
    • fiftyone/server/mutation.py
    • docs/source/tutorials/pandas_comparison.ipynb
    • docs/source/recipes/creating_views.ipynb
    • docs/source/user_guide/using_views.rst
    • docs/source/cheat_sheets/pandas_vs_fiftyone.rst
    • docs/source/cheat_sheets/views_cheat_sheet.rst
    • docs/source/cheat_sheets/filtering_cheat_sheet.rst
    • fiftyone/core/video.py
    • fiftyone/core/stages.py
    • fiftyone/core/patches.py
    • fiftyone/core/frame.py
    • fiftyone/core/document.py
    • fiftyone/core/dataset.py
    • fiftyone/core/clips.py
    • fiftyone/core/collections.py
    • fiftyone/core/sample.py
    • fiftyone/core/odm/mixins.py
    • fiftyone/core/odm/document.py
    • fiftyone/core/aggregations.py
    • tests/intensive/patches_tests.py
    • tests/intensive/merge_tests.py
    • tests/intensive/evaluation_tests.py
    • tests/intensive/cvat_tests.py
    • fiftyone/core/expressions.py
    • tests/intensive/collections_tests.py
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all method calls to `set_field` correctly handle read-only fields.

# Test: Search for the method usage. Expect: Correct handling of read-only fields.
rg --type python -A 5 $'set_field'

Length of output: 64


Script:

#!/bin/bash
# Description: Verify all method calls to `set_field` correctly handle read-only fields.

# Test: Search for the method usage. Expect: Correct handling of read-only fields.
rg -A 5 $'set_field'

Length of output: 94089


751-765: LGTM! But verify the method usage in the codebase.

The addition of the virtual parameter optimizes the update logic. Ensure that all calls to _update correctly handle the new virtual parameter.

Verification successful

The virtual parameter is correctly handled in the _update method.

The _update method in both fiftyone/core/odm/document.py and fiftyone/core/odm/mixins.py includes the virtual parameter and correctly skips updating the last_modified_at field when virtual is True.

  • fiftyone/core/odm/document.py
  • fiftyone/core/odm/mixins.py
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all method calls to `_update` match the new signature.

# Test: Search for the method usage. Expect: Only occurrences of the new signature.
rg --type python -A 5 $'_update'

Length of output: 62


Script:

#!/bin/bash
# Description: Verify all method calls to `_update` match the new signature.

# Test: Search for the method usage. Expect: Only occurrences of the new signature.
rg --type py -A 5 '_update'

Length of output: 70749


Script:

#!/bin/bash
# Description: Verify the handling of the `virtual` parameter in the `_update` method.

# Extract the definition and relevant lines of `_update` from `fiftyone/core/odm/mixins.py` and `fiftyone/core/odm/document.py`.
rg --type py -A 10 'def _update' fiftyone/core/odm/mixins.py fiftyone/core/odm/document.py

Length of output: 3864

tests/unittests/patches_tests.py (4)

754-950: LGTM!

The new method test_to_patches_datetimes ensures that the timestamp fields created_at and last_modified_at are correctly validated and immutable.


Line range hint 75-102: LGTM! But verify the test method usage in the codebase.

The changes ensure that the new fields created_at and last_modified_at are correctly validated in the schema. Ensure that all calls to test_to_patches correctly handle the new fields.


423-429: LGTM! But verify the test method usage in the codebase.

The changes ensure that the new fields created_at and last_modified_at are correctly validated in the index structure. Ensure that all calls to test_to_evaluation_patches correctly handle the new fields.

Verification successful

Verified the test method usage in the codebase.

The fields created_at and last_modified_at are correctly handled and validated within the test_to_evaluation_patches method and other relevant test methods.

  • tests/unittests/patches_tests.py: Extensive usage and validation of created_at and last_modified_at.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all method calls to `test_to_evaluation_patches` correctly handle the new fields.

# Test: Search for the method usage. Expect: Correct handling of the new fields.
rg --type python -A 5 $'test_to_evaluation_patches'

Length of output: 81


Script:

#!/bin/bash
# Description: Verify all method calls to `test_to_evaluation_patches` correctly handle the new fields.

# Test: Search for the method usage. Expect: Correct handling of the new fields.
rg -A 5 'test_to_evaluation_patches'

Length of output: 806


Script:

#!/bin/bash
# Description: Gather more lines of context around the `test_to_evaluation_patches` method to verify correct handling of new fields.

# Test: Search for the method usage and gather additional lines of context.
rg -A 20 -B 5 'test_to_evaluation_patches' tests/unittests/patches_tests.py tests/intensive/patches_tests.py

Length of output: 3527


Script:

#!/bin/bash
# Description: Search for the usage of `created_at` and `last_modified_at` within the relevant test files to verify correct handling.

# Test: Search for `created_at` and `last_modified_at` usage.
rg 'created_at|last_modified_at' tests/unittests/patches_tests.py tests/intensive/patches_tests.py

Length of output: 5658


110-116: LGTM! But verify the test method usage in the codebase.

The changes ensure that the new fields created_at and last_modified_at are correctly validated in the index structure. Ensure that all calls to test_to_patches correctly handle the new fields.

Verification successful

All calls to test_to_patches correctly handle the new fields.

The methods test_to_patches and test_to_patches_datetimes in tests/unittests/patches_tests.py and test_to_patches in tests/intensive/patches_tests.py have been verified to include the handling of the new fields created_at and last_modified_at.

  • tests/unittests/patches_tests.py:

    • test_to_patches(self) verifies the presence of created_at and last_modified_at in the index structure.
    • test_to_patches_datetimes(self) includes assertions and checks related to the created_at and last_modified_at fields.
  • tests/intensive/patches_tests.py:

    • test_to_patches() includes operations and checks that ensure the correct handling of the dataset and its fields.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all method calls to `test_to_patches` correctly handle the new fields.

# Test: Search for the method usage. Expect: Correct handling of the new fields.
rg --type python -A 5 $'test_to_patches'

Length of output: 70


Script:

#!/bin/bash
# Description: Verify all method calls to `test_to_patches` correctly handle the new fields.

# Test: Search for the method usage. Expect: Correct handling of the new fields.
rg -A 5 'test_to_patches'

Length of output: 1137


Script:

#!/bin/bash
# Description: Extract the full definitions of the `test_to_patches` methods to verify handling of new fields.

# Extract the full definition of `test_to_patches` in `tests/unittests/patches_tests.py`
rg -A 20 'def test_to_patches(self):' tests/unittests/patches_tests.py

# Extract the full definition of `test_to_patches_datetimes` in `tests/unittests/patches_tests.py`
rg -A 20 'def test_to_patches_datetimes(self):' tests/unittests/patches_tests.py

# Extract the full definition of `test_to_patches` in `tests/intensive/patches_tests.py`
rg -A 20 'def test_to_patches():' tests/intensive/patches_tests.py

Length of output: 224


Script:

#!/bin/bash
# Description: Extract the full definitions of the `test_to_patches` methods to verify handling of new fields.

# Extract the full definition of `test_to_patches` in `tests/unittests/patches_tests.py`
rg -A 50 'def test_to_patches(self):' tests/unittests/patches_tests.py

# Extract the full definition of `test_to_patches_datetimes` in `tests/unittests/patches_tests.py`
rg -A 50 'def test_to_patches_datetimes(self):' tests/unittests/patches_tests.py

# Extract the full definition of `test_to_patches` in `tests/intensive/patches_tests.py`
rg -A 50 'def test_to_patches():' tests/intensive/patches_tests.py

Length of output: 224


Script:

#!/bin/bash
# Description: Extract the full definitions of the `test_to_patches` methods to verify handling of new fields.

# Extract the full definition of `test_to_patches` in `tests/unittests/patches_tests.py`
ast-grep --lang python --pattern 'def test_to_patches(self): $$$' tests/unittests/patches_tests.py

# Extract the full definition of `test_to_patches_datetimes` in `tests/unittests/patches_tests.py`
ast-grep --lang python --pattern 'def test_to_patches_datetimes(self): $$$' tests/unittests/patches_tests.py

# Extract the full definition of `test_to_patches` in `tests/intensive/patches_tests.py`
ast-grep --lang python --pattern 'def test_to_patches(): $$$' tests/intensive/patches_tests.py

Length of output: 40600

fiftyone/core/patches.py (2)

870-871: LGTM! But verify the function usage in the codebase.

The changes expand the metadata available for each patch by including created_at and last_modified_at. Ensure that all calls to _make_patches_view correctly handle the new fields.

Verification successful

The changes correctly handle the created_at and last_modified_at fields.

The _make_patches_view function includes these fields in the projection stage of the MongoDB aggregation pipeline, ensuring they are part of the patches view.

  • fiftyone/core/patches.py, lines 870-871
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `_make_patches_view` correctly handle the new fields.

# Test: Search for the function usage. Expect: Correct handling of the new fields.
rg --type python -A 5 $'_make_patches_view'

Length of output: 73


Script:

#!/bin/bash
# Description: Verify all function calls to `_make_patches_view` correctly handle the new fields.

# Test: Search for the function usage. Expect: Correct handling of the new fields.
rg --type py -A 5 '_make_patches_view'

Length of output: 1042


Script:

#!/bin/bash
# Description: Extract more context around the `_make_patches_view` function usage to verify handling of `created_at` and `last_modified_at`.

# Test: Get more lines of context around the function usage.
rg --type py -A 10 -B 10 '_make_patches_view'

Length of output: 3546


Script:

#!/bin/bash
# Description: Extract the full implementation of the `_make_patches_view` function to verify handling of `created_at` and `last_modified_at`.

# Test: Get the full implementation of the `_make_patches_view` function.
rg --type py -A 50 -B 10 'def _make_patches_view'

Length of output: 3438


636-640: LGTM! But verify the function usage in the codebase.

The changes enhance clarity by specifying the field and its depth in the exception message. Ensure that all calls to make_patches_dataset correctly handle the new constraint.

Verification successful

The function make_patches_dataset is correctly handling the new constraint on the depth of nested fields. The test case in tests/unittests/patches_tests.py ensures this functionality is tested.

  • tests/unittests/patches_tests.py: The test case test_make_patches_dataset verifies the functionality of make_patches_dataset.
  • fiftyone/core/stages.py: The function call correctly passes the field parameter, and the constraint is managed within the function.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `make_patches_dataset` correctly handle the new constraint.

# Test: Search for the function usage. Expect: Correct handling of the new constraint.
rg --type python -A 5 $'make_patches_dataset'

Length of output: 75


Script:

#!/bin/bash
# Description: Verify all function calls to `make_patches_dataset` correctly handle the new constraint.

# Test: Search for the function usage. Expect: Correct handling of the new constraint.
rg -t py -A 5 $'make_patches_dataset'

Length of output: 2030


Script:

#!/bin/bash
# Extract relevant portions of tests/unittests/patches_tests.py to verify handling of the new constraint
rg -A 10 'def test_make_patches_dataset' tests/unittests/patches_tests.py

# Extract relevant portions of fiftyone/core/stages.py to verify handling of the new constraint
rg -A 10 'make_patches_dataset' fiftyone/core/stages.py

Length of output: 1415


Script:

#!/bin/bash
# Extract more context around the call to make_patches_dataset in tests/unittests/patches_tests.py
rg -A 20 -B 5 'make_patches_dataset' tests/unittests/patches_tests.py

# Extract more context around the call to make_patches_dataset in fiftyone/core/stages.py
rg -A 20 -B 5 'make_patches_dataset' fiftyone/core/stages.py

Length of output: 3697

fiftyone/core/video.py (3)

176-176: LGTM! The exclusion of "last_modified_at" field is appropriate.

The change ensures that "last_modified_at" is not included in operations involving sample-specific fields.


383-383: LGTM! Including "last_modified_at" field in the projection is appropriate.

The change ensures that "last_modified_at" is included in the output of the pipeline, enhancing visibility of this field.


641-641: LGTM! But be cautious with overwrite=True in merge_field_schema.

The change allows existing field schemas to be replaced with those from the frame_schema, leading to more flexible schema management. However, it also risks overwriting existing configurations if not handled carefully.

docs/source/user_guide/groups.rst (10)

126-132: LGTM! The addition of created_at and last_modified_at fields is appropriate.

The change enhances the semantic richness of the Sample object representation by including timestamps for creation and modification times.


218-219: LGTM! The addition of created_at and last_modified_at fields is appropriate.

The change provides a more comprehensive view of the attributes associated with each sample.


301-307: LGTM! The addition of created_at and last_modified_at fields is appropriate.

The change ensures that the documentation reflects the new fields added to the Sample object.


364-365: LGTM! The addition of created_at and last_modified_at fields is appropriate.

The change provides a more comprehensive view of the attributes associated with each sample.


374-375: LGTM! The addition of created_at and last_modified_at fields is appropriate.

The change provides a more comprehensive view of the attributes associated with each sample.


384-385: LGTM! The addition of created_at and last_modified_at fields is appropriate.

The change provides a more comprehensive view of the attributes associated with each sample.


451-452: LGTM! The addition of created_at and last_modified_at fields is appropriate.

The change provides a more comprehensive view of the attributes associated with each sample.


548-553: LGTM! The addition of created_at and last_modified_at fields is appropriate.

The change ensures that the documentation reflects the new fields added to the Sample object.


836-841: LGTM! The addition of created_at and last_modified_at fields is appropriate.

The change ensures that the documentation reflects the new fields added to the Sample object.


861-866: LGTM! The addition of created_at and last_modified_at fields is appropriate.

The change ensures that the documentation reflects the new fields added to the Sample object.

fiftyone/core/clips.py (4)

819-820: LGTM! Including created_at and last_modified_at fields in the projection is appropriate.

The change enhances the functionality by including timestamps that track the creation and modification times of the clips being processed.


868-869: LGTM! Including created_at and last_modified_at fields in the projection is appropriate.

The change enhances the functionality by including timestamps that track the creation and modification times of the clips being processed.


942-943: LGTM! Including created_at and last_modified_at fields in the projection is appropriate.

The change enhances the functionality by including timestamps that track the creation and modification times of the clips being processed.


1028-1029: LGTM! Including created_at and last_modified_at fields in the projection is appropriate.

The change enhances the functionality by including timestamps that track the creation and modification times of the clips being processed.

fiftyone/core/frame.py (5)

306-311: LGTM! The _enforce_read_only parameter is correctly added.

The addition of the _enforce_read_only parameter ensures that read-only constraints are enforced when setting the frame number.


Line range hint 649-671:
LGTM! The created_at and last_modified_at parameters are correctly used.

The addition of created_at and last_modified_at parameters ensures that the timestamps are correctly set in the dictionary.


Line range hint 723-792:
LGTM! The **kwargs addition and timestamp parameters are correctly used.

The addition of **kwargs provides flexibility for future extensions. The use of created_at and last_modified_at parameters ensures that timestamps are correctly set.

Tools
Ruff

814-818: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)


798-824: LGTM! The _validate_frame function correctly validates individual frames.

The function correctly validates individual frames and raises detailed error messages if validation fails.

Tools
Ruff

814-818: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)


907-921: LGTM! The _enforce_read_only parameter is correctly added.

The addition of the _enforce_read_only parameter ensures that read-only constraints are enforced when setting the frame number.

fiftyone/core/fields.py (12)

Line range hint 59-111:
LGTM! The read_only parameter is correctly validated.

The addition of the read_only parameter and its validation ensures that the constraints are correctly enforced.


Line range hint 112-148:
LGTM! The read_only parameter is correctly used.

The addition of the read_only parameter ensures that the field matches the read-only constraint.


Line range hint 152-214:
LGTM! The read_only parameter is correctly used.

The addition of the read_only parameter ensures that the field matches the read-only constraint.


223-239: LGTM! The get_field_metadata function correctly retrieves metadata.

The function correctly retrieves editable metadata for fields, including the read_only status.


Line range hint 240-283:
LGTM! The read_only parameter is correctly used.

The addition of the read_only parameter ensures that fields are filtered based on their read-only status.


Line range hint 290-327:
LGTM! The read_only parameter is correctly used.

The addition of the read_only parameter ensures that fields are filtered based on their read-only status.


Line range hint 338-459:
LGTM! The read_only parameter and property are correctly added.

The addition of the read_only parameter and property ensures that the Field class handles the read-only status correctly.


516-523: LGTM! The read_only parameter is correctly added.

The addition of the read_only parameter ensures that the IntField class handles the read-only status correctly.


538-545: LGTM! The read_only parameter is correctly added.

The addition of the read_only parameter ensures that the ObjectIdField class handles the read-only status correctly.


566-573: LGTM! The read_only parameter is correctly added.

The addition of the read_only parameter ensures that the UUIDField class handles the read-only status correctly.


582-589: LGTM! The read_only parameter is correctly added.

The addition of the read_only parameter ensures that the BooleanField class handles the read-only status correctly.


602-609: LGTM! The read_only parameter is correctly added.

The addition of the read_only parameter ensures that the DateField class handles the read-only status correctly.

fiftyone/core/odm/mixins.py (8)

133-138: LGTM!

The addition of the _enforce_read_only parameter to enforce read-only constraints is a good enhancement.


Line range hint 178-221:
LGTM!

The addition of the read_only parameter to filter the schema based on read-only status is a useful enhancement.


Line range hint 234-313:
LGTM!

The addition of the overwrite parameter to allow overwriting field metadata is a useful enhancement.

Tools
Ruff

296-296: Use key in dict instead of key in dict.keys()

Remove .keys()

(SIM118)


Line range hint 339-390:
LGTM!

The addition of the read_only parameter to allow marking fields as read-only is a useful enhancement.


Line range hint 454-466:
LGTM!

The addition of the read_only parameter to allow marking fields as read-only is a useful enhancement.


Line range hint 1059-1191:
LGTM!

The addition of the overwrite parameter to allow overwriting field metadata is a useful enhancement.


1433-1439: LGTM!

The auto-population of created_at and last_modified_at fields during insertion is a good enhancement.


1447-1457: LGTM!

The update of the last_modified_at field during modifications is a good enhancement.

fiftyone/core/view.py (7)

902-902: Ensure the read_only parameter is passed correctly.

The read_only parameter is correctly passed to the _dataset.get_field_schema method.


913-913: Ensure the read_only parameter is passed correctly in the flattened schema.

The read_only parameter is correctly passed to the fof.flatten_schema method.


957-957: Ensure the read_only parameter is passed correctly.

The read_only parameter is correctly passed to the _dataset.get_frame_field_schema method.


968-968: Ensure the read_only parameter is passed correctly in the flattened schema.

The read_only parameter is correctly passed to the fof.flatten_schema method.


1795-1810: Ensure proper initialization and updating of edited_fields.

The method correctly initializes and updates the edited_fields set.


923-923: Add a default value for the read_only parameter.

The read_only parameter should have a default value, such as None, to maintain backward compatibility and clarify its optional nature.

-    read_only=None,
+    read_only=None,

Likely invalid or redundant comment.


874-874: Add a default value for the read_only parameter.

The read_only parameter should have a default value, such as None, to maintain backward compatibility and clarify its optional nature.

-    read_only=None,
+    read_only=None,

Likely invalid or redundant comment.

tests/unittests/group_tests.py (5)

1424-1425: LGTM!

The created_at and last_modified_at fields are correctly added to the group_indexes set.


1427-1428: LGTM!

The frames.created_at and frames.last_modified_at fields are correctly added to the group_indexes set.


1670-1672: LGTM!

The default_indexes variable correctly consolidates the common index fields including the newly added created_at and last_modified_at fields.


1749-1755: LGTM!

The assertions are correctly updated to incorporate default_indexes, ensuring consistency with the new index structure.


1839-1840: LGTM!

The assertions are correctly updated to incorporate default_indexes, ensuring consistency with the new index structure.

Also applies to: 1846-1846, 1860-1860, 1868-1868

docs/source/user_guide/app.rst (2)

2450-2450: Addition of created_at field looks good.

The created_at field is correctly documented and integrated.


2451-2451: Addition of last_modified_at field looks good.

The last_modified_at field is correctly documented and integrated.

docs/source/user_guide/evaluation.rst (1)

1970-1971: Verify the correctness and placement of the new fields.

The created_at and last_modified_at fields have been added. Ensure these fields are correctly documented and placed in the appropriate section of the documentation.

docs/source/user_guide/using_views.rst (4)

52-53: Fields added correctly.

The created_at and last_modified_at fields are correctly added to the Sample fields section.


1110-1111: Fields added correctly.

The created_at and last_modified_at fields are correctly added to the Patch fields section.


1391-1392: Fields added correctly.

The created_at and last_modified_at fields are correctly added to the Clip fields section.


1397-1398: Fields added correctly.

The created_at and last_modified_at fields are correctly added to the Frame fields section.

Also applies to: 1502-1503, 1664-1665, 1732-1733

fiftyone/utils/data/importers.py (3)

8-8: Import statement approved.

The import of the datetime module is necessary for the new timestamp functionality.


1910-1910: Set created_at and last_modified_at fields for samples.

The created_at and last_modified_at fields are set to the current UTC time using datetime.utcnow(). This ensures that each sample has consistent timestamp information.


1924-1927: Set created_at and last_modified_at fields for frames.

The created_at and last_modified_at fields are set to the current UTC time using datetime.utcnow(). This ensures that each frame has consistent timestamp information.

tests/unittests/import_export_tests.py (3)

3622-3649: LGTM! Ensure the function usage in the codebase.

The code changes are approved.

However, ensure that all function calls to test_legacy_fiftyone_dataset are correctly implemented.


3270-3297: LGTM! Ensure the function usage in the codebase.

The code changes are approved.

However, ensure that all function calls to test_fiftyone_dataset are correctly implemented.


4720-4753: LGTM! Ensure the function usage in the codebase.

The code changes are approved.

However, ensure that all function calls to test_legacy_fiftyone_dataset are correctly implemented.

Verification successful

Function test_legacy_fiftyone_dataset is correctly defined and implemented in the codebase.

  • The function test_legacy_fiftyone_dataset is defined in multiple test files and appears to be used as a test case.
  • There are no direct calls to this function, suggesting it is executed by a test runner.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `test_legacy_fiftyone_dataset` match the new implementation.

# Test: Search for the function usage. Expect: Only occurrences of the new implementation.
rg --type python -A 5 $'test_legacy_fiftyone_dataset'

Length of output: 83


Script:

#!/bin/bash
# Description: Verify all function calls to `test_legacy_fiftyone_dataset` match the new implementation.

# Test: Search for the function usage. Expect: Only occurrences of the new implementation.
rg --type py -A 5 'test_legacy_fiftyone_dataset'

Length of output: 2156

tests/unittests/view_tests.py (21)

60-61: Ensure last_modified_at captures initial state correctly.

The last_modified_at1 variable captures the initial state of the last_modified_at field. Ensure this is done before any modifications to maintain accuracy.


66-67: Capture last_modified_at after first batch of updates.

The last_modified_at2 variable captures the state after the first batch of updates. Ensure the updates are correctly reflected in the timestamps.


70-74: Assert increasing order of timestamps.

The assertion checks that the timestamps are in strictly increasing order. This ensures that the last_modified_at field is correctly updated.


86-87: Capture last_modified_at after second batch of updates.

The last_modified_at3 variable captures the state after the second batch of updates. Ensure the updates are correctly reflected in the timestamps.


90-94: Assert increasing order of timestamps after second batch.

The assertion checks that the timestamps are in strictly increasing order after the second batch of updates. This ensures that the last_modified_at field is correctly updated.


101-102: Capture last_modified_at after third batch of updates.

The last_modified_at4 variable captures the state after the third batch of updates. Ensure the updates are correctly reflected in the timestamps.


105-109: Assert increasing order of timestamps after third batch.

The assertion checks that the timestamps are in strictly increasing order after the third batch of updates. This ensures that the last_modified_at field is correctly updated.


1732-1733: New test function for last_modified_at updates.

The test_set_values_last_modified_at function is a new addition. Ensure it comprehensively tests the last_modified_at field updates.


1746-1747: Capture initial last_modified_at state.

The lma1 variable captures the initial state of the last_modified_at field. Ensure this is done before any modifications to maintain accuracy.


1750-1751: Capture last_modified_at after setting values.

The lma2 variable captures the state after setting values. Ensure the updates are correctly reflected in the timestamps.


1752-1755: Assert correct timestamp updates.

The assertion checks that the timestamps are correctly updated. This ensures that the last_modified_at field is functioning as expected.


1759-1760: Capture initial last_modified_at state for view.

The lma1 variable captures the initial state of the last_modified_at field for the view. Ensure this is done before any modifications to maintain accuracy.


1764-1765: Capture last_modified_at after setting values in view.

The lma2 variable captures the state after setting values in the view. Ensure the updates are correctly reflected in the timestamps.


1766-1769: Assert correct timestamp updates for view.

The assertion checks that the timestamps are correctly updated for the view. This ensures that the last_modified_at field is functioning as expected.


1771-1772: New test function for last_modified_at updates in video frames.

The test_set_values_video_last_modified_at function is a new addition. Ensure it comprehensively tests the last_modified_at field updates in video frames.


1789-1790: Capture initial last_modified_at state for frames.

The lma1 variable captures the initial state of the last_modified_at field for frames. Ensure this is done before any modifications to maintain accuracy.


1793-1794: Capture last_modified_at after setting values in frames.

The lma2 variable captures the state after setting values in frames. Ensure the updates are correctly reflected in the timestamps.


1795-1798: Assert correct timestamp updates for frames.

The assertion checks that the timestamps are correctly updated for frames. This ensures that the last_modified_at field is functioning as expected.


1802-1803: Capture initial last_modified_at state for frames view.

The lma1 variable captures the initial state of the last_modified_at field for the frames view. Ensure this is done before any modifications to maintain accuracy.


1807-1808: Capture last_modified_at after setting values in frames view.

The lma2 variable captures the state after setting values in the frames view. Ensure the updates are correctly reflected in the timestamps.


1809-1812: Assert correct timestamp updates for frames view.

The assertion checks that the timestamps are correctly updated for the frames view. This ensures that the last_modified_at field is functioning as expected.

docs/source/user_guide/using_datasets.rst (28)

973-1001: Document the new default fields clearly.

The new created_at and last_modified_at fields are well-documented, but ensure that users understand their read-only nature and automatic population.


1002-1006: Highlight the read-only nature of created_at and last_modified_at fields.

The note clarifies that these fields are read-only and automatically populated/updated, which is crucial information for users.


1025-1026: Ensure new fields are included in examples.

The examples correctly include the new created_at and last_modified_at fields, demonstrating their usage.

Also applies to: 1040-1040


1074-1076: Include new fields in schema examples.

The schema examples correctly include the new created_at and last_modified_at fields, ensuring users understand their presence in the schema.

Also applies to: 1095-1100


1136-1142: Ensure new fields are included in examples.

The examples correctly include the new created_at and last_modified_at fields in the schema, demonstrating their presence when adding new fields.


1437-1447: Ensure new fields are included in metadata examples.

The examples correctly include the new created_at and last_modified_at fields, demonstrating their presence when storing field metadata.


1508-1515: Document the read-only nature of new fields clearly.

The section correctly documents the read-only nature of the created_at and last_modified_at fields, providing clear examples.


1516-1531: Provide clear examples for read-only fields.

The examples clearly demonstrate the read-only nature of the created_at and last_modified_at fields, showing what happens when users try to modify them.


1533-1564: Document how to mark additional fields as read-only.

The section provides clear instructions and examples on how to mark additional fields as read-only, enhancing user control over field accessibility.


1566-1571: Ensure users save changes after marking fields as read-only.

The note correctly emphasizes the need to save changes after marking fields as read-only, ensuring that the changes are persisted.


1573-1584: Clarify that read-only fields do not interfere with sample operations.

The section clarifies that read-only fields do not interfere with adding or deleting samples, providing important context for users.


1585-1605: Provide examples for reverting read-only fields to editable.

The examples clearly demonstrate how to revert read-only fields to editable, providing flexibility for users.


1870-1871: Include new fields in datetime examples.

The examples correctly include the new created_at and last_modified_at fields, demonstrating their usage with datetime values.

Also applies to: 1875-1876


Line range hint 1900-1909:
Ensure new fields are included in datetime examples.

The examples correctly include the new created_at and last_modified_at fields, demonstrating their usage with datetime values.


2000-2001: Include new fields in regression examples.

The examples correctly include the new created_at and last_modified_at fields, demonstrating their presence in regression samples.


2056-2057: Include new fields in classification examples.

The examples correctly include the new created_at and last_modified_at fields, demonstrating their presence in classification samples.


2127-2128: Include new fields in multilabel classification examples.

The examples correctly include the new created_at and last_modified_at fields, demonstrating their presence in multilabel classification samples.


2246-2247: Include new fields in object detection examples.

The examples correctly include the new created_at and last_modified_at fields, demonstrating their presence in object detection samples.


2367-2368: Include new fields in instance segmentation examples.

The examples correctly include the new created_at and last_modified_at fields, demonstrating their presence in instance segmentation samples.


2503-2504: Include new fields in polyline examples.

The examples correctly include the new created_at and last_modified_at fields, demonstrating their presence in polyline samples.


2910-2911: Include new fields in semantic segmentation examples.

The examples correctly include the new created_at and last_modified_at fields, demonstrating their presence in semantic segmentation samples.


3012-3013: Include new fields in heatmap examples.

The examples correctly include the new created_at and last_modified_at fields, demonstrating their presence in heatmap samples.


3172-3173: Include new fields in temporal detection examples.

The examples correctly include the new created_at and last_modified_at fields, demonstrating their presence in temporal detection samples.


3425-3426: Include new fields in geolocation examples.

The examples correctly include the new created_at and last_modified_at fields, demonstrating their presence in geolocation samples.


4273-4274: Include new fields in custom embedded document examples.

The examples correctly include the new created_at and last_modified_at fields, demonstrating their presence in custom embedded documents.


4324-4325: Include new fields in image dataset examples.

The examples correctly include the new created_at and last_modified_at fields, demonstrating their presence in image datasets.


4383-4384: Include new fields in video dataset examples.

The examples correctly include the new created_at and last_modified_at fields, demonstrating their presence in video datasets.


5024-5025: Include new fields in point cloud dataset examples.

The examples correctly include the new created_at and last_modified_at fields, demonstrating their presence in point cloud datasets.

tests/unittests/dataset_tests.py (4)

481-535: LGTM!

The function test_last_modified_at correctly tests the behavior of the last_modified_at field during various dataset operations.


2332-2487: LGTM!

The function test_read_only_fields thoroughly tests the behavior of read-only fields, ensuring they cannot be modified directly by users.


2489-2665: LGTM!

The function test_read_only_frame_fields thoroughly tests the behavior of read-only fields in frames, ensuring they cannot be modified directly by users.


5333-5368: LGTM!

The function test_set_new_fields correctly tests the ability to set new fields on samples and ensure they are correctly added to the dataset schema.

fiftyone/core/stages.py (9)

101-117: Placeholder function approved.

The get_edited_fields function in the ViewStage class is a placeholder and currently returns None. This is acceptable for an abstract base class.


1421-1442: LGTM!

The get_edited_fields function in the ExcludeLabels class correctly identifies and returns the fields that may have been edited by the stage.


1828-1837: LGTM!

The get_edited_fields function in the FilterField class correctly handles the frame field check and returns the appropriate field name if applicable.


2359-2368: LGTM!

The get_edited_fields function in the FilterLabels class correctly handles the frame field check and returns the appropriate field name if applicable.


2782-2791: LGTM!

The get_edited_fields function in the FilterKeypoints class correctly handles the frame field check and returns the appropriate field name if applicable.


3971-3980: LGTM!

The get_edited_fields function in the LimitLabels class correctly handles the frame field check and returns the appropriate field name if applicable.


4144-4153: LGTM!

The get_edited_fields function in the MapLabels class correctly handles the frame field check and returns the appropriate field name if applicable.


4320-4329: LGTM!

The get_edited_fields function in the SetField class correctly handles the frame field check and returns the appropriate field name if applicable.


6545-6566: LGTM!

The get_edited_fields function in the SelectLabels class correctly identifies and returns the fields that may have been edited by the stage.

fiftyone/core/dataset.py (7)

690-692: LGTM!

The last_modified_at property is correctly added to the Dataset class.


Line range hint 1220-1257:
LGTM!

The read_only parameter is correctly integrated into the get_field_schema method.


Line range hint 1267-1310:
LGTM!

The read_only parameter is correctly integrated into the get_frame_field_schema method.


Line range hint 1325-1373:
LGTM!

The read_only parameter is correctly integrated into the add_sample_field method.


Line range hint 1471-1516:
LGTM!

The read_only parameter is correctly integrated into the add_frame_field method.


Line range hint 1614-1638:
LGTM!

The read_only parameter is correctly integrated into the add_group_field method.


Line range hint 2787-2808:
LGTM!

The created_at and last_modified_at parameters are correctly integrated into the _make_dict method.

fiftyone/core/collections.py (30)

11-11: Import statement is correct.

The import of datetime is necessary for timestamp functionality.


571-585: Method sync_last_modified_at looks good.

The method is well-documented and handles the synchronization of last_modified_at property effectively.


586-589: Method _sync_samples_last_modified_at looks good.

The method correctly handles the synchronization of sample timestamps.


1233-1235: Method _is_read_only_field looks good.

The method correctly determines if a field is read-only.


1237-1254: Method get_field looks good.

The addition of the read_only parameter is consistent with the new functionality.


1287-1302: Field validation logic looks good.

The calls to validate_constraints and validate_field ensure that the field matches the provided constraints.


1311-1311: Method _parse_field looks good.

The method correctly handles the parsing of fields, including read-only status.


1333-1335: Method _parse_field looks good.

The method continues to correctly handle the parsing of fields, including read-only status.


1343-1346: Method _parse_field looks good.

The method continues to correctly handle the parsing of fields, including read-only status.


1362-1362: Method _parse_field looks good.

The method continues to correctly handle the parsing of fields, including read-only status.


Line range hint 1368-1386:
Method get_field_schema looks good.

The addition of the read_only parameter is consistent with the new functionality.


1399-1399: Method get_field_schema looks good.

The method continues to correctly handle the parsing of fields, including read-only status.


1415-1416: Method get_field_schema looks good.

The method continues to correctly handle the parsing of fields, including read-only status.


1797-1801: Method _edit_sample_tags looks good.

The method correctly checks for read-only fields and updates the last_modified_at timestamp.


1840-1846: Method _edit_label_tags looks good.

The method correctly checks for read-only fields and updates the last_modified_at timestamp.


1900-1906: Method _edit_label_tags looks good.

The method continues to correctly check for read-only fields and updates the last_modified_at timestamp.


1926-1931: Method _edit_label_tags looks good.

The method continues to correctly check for read-only fields and updates the last_modified_at timestamp.


1941-1941: Method _edit_label_tags looks good.

The method continues to correctly check for read-only fields and updates the last_modified_at timestamp.


1955-1967: Method _edit_label_tags looks good.

The method continues to correctly check for read-only fields and updates the last_modified_at timestamp.


2396-2398: Method _set_values looks good.

The method correctly checks for read-only fields.


2555-2557: Method _set_values looks good.

The method continues to correctly check for read-only fields.


2907-2911: Method _set_doc_values looks good.

The method correctly checks for read-only fields and updates the last_modified_at timestamp.


2925-2930: Method _set_doc_values looks good.

The method continues to correctly check for read-only fields and updates the last_modified_at timestamp.


2950-2954: Method _set_list_values_by_id looks good.

The method correctly checks for read-only fields and updates the last_modified_at timestamp.


2991-2991: Method _set_list_values_by_id looks good.

The method continues to correctly check for read-only fields and updates the last_modified_at timestamp.


3012-3016: Method _set_label_list_values looks good.

The method correctly checks for read-only fields and updates the last_modified_at timestamp.


3037-3037: Method _set_label_list_values looks good.

The method continues to correctly check for read-only fields and updates the last_modified_at timestamp.


3048-3058: Method _set_labels looks good.

The method correctly checks for read-only fields and updates the last_modified_at timestamp.


3083-3083: Method _set_labels looks good.

The method continues to correctly


591-634: Method _sync_dataset_last_modified_at looks good, but avoid bare except.

The method correctly handles the synchronization of dataset timestamps. However, using a bare except is not recommended. Specify the exception type to catch.

-        except:
+        except StopIteration:

Likely invalid or redundant comment.

fiftyone/core/odm/dataset.py Outdated Show resolved Hide resolved
fiftyone/core/odm/mixins.py Show resolved Hide resolved
fiftyone/core/odm/mixins.py Show resolved Hide resolved
fiftyone/core/fields.py Show resolved Hide resolved
fiftyone/core/fields.py Outdated Show resolved Hide resolved
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: 7

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 2bf6843 and 7774662.

Files selected for processing (4)
  • fiftyone/core/frame.py (10 hunks)
  • fiftyone/migrations/revisions/v0_26_0.py (1 hunks)
  • package/desktop/setup.py (1 hunks)
  • setup.py (2 hunks)
Files skipped from review as they are similar to previous changes (3)
  • fiftyone/core/frame.py
  • package/desktop/setup.py
  • setup.py

fiftyone/migrations/revisions/v0_26_0.py Outdated Show resolved Hide resolved
fiftyone/migrations/revisions/v0_26_0.py Outdated Show resolved Hide resolved
fiftyone/migrations/revisions/v0_26_0.py Outdated Show resolved Hide resolved
fiftyone/migrations/revisions/v0_26_0.py Outdated Show resolved Hide resolved
fiftyone/migrations/revisions/v0_26_0.py Outdated Show resolved Hide resolved
fiftyone/migrations/revisions/v0_26_0.py Outdated Show resolved Hide resolved
fiftyone/migrations/revisions/v0_26_0.py Outdated Show resolved Hide resolved
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 7774662 and 09719e8.

Files selected for processing (1)
  • fiftyone/migrations/revisions/v0_26_0.py (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • fiftyone/migrations/revisions/v0_26_0.py


def _add_created_at(db, dataset_name, collection_name):
try:
pipeline = [{"$set": {"created_at": {"$toDate": "$_id"}}}]
Copy link
Contributor

Choose a reason for hiding this comment

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

Pulling the timestamp from the id. Very cool 🚀

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: 5

Outside diff range, codebase verification and nitpick comments (3)
tests/unittests/dataset_tests.py (1)

481-535: Consider adding comments for clarity.

Adding comments to explain the purpose of timestamp comparisons would improve readability and maintainability.

+        # Ensure timestamps are correctly updated after modifying dataset info
fiftyone/core/dataset.py (2)

Line range hint 7294-7299: Avoid bare except clauses.

Use a specific exception type instead of a bare except to improve error handling clarity.

-  except:
+  except Exception:
Tools
Ruff

7281-7281: Do not use bare except

(E722)


7295-7295: Do not use bare except

(E722)


Line range hint 7294-7299: Avoid bare except clauses.

Use a specific exception type instead of a bare except to improve error handling clarity.

-  except:
+  except Exception:
Tools
Ruff

7281-7281: Do not use bare except

(E722)


7295-7295: Do not use bare except

(E722)

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 09719e8 and 9ef797d.

Files selected for processing (12)
  • docs/source/user_guide/using_datasets.rst (41 hunks)
  • fiftyone/core/collections.py (34 hunks)
  • fiftyone/core/dataset.py (78 hunks)
  • fiftyone/core/fields.py (46 hunks)
  • fiftyone/core/frame.py (9 hunks)
  • fiftyone/core/odm/dataset.py (4 hunks)
  • fiftyone/core/odm/mixins.py (29 hunks)
  • fiftyone/core/odm/utils.py (8 hunks)
  • fiftyone/core/utils.py (2 hunks)
  • fiftyone/core/view.py (18 hunks)
  • fiftyone/utils/data/importers.py (7 hunks)
  • tests/unittests/dataset_tests.py (48 hunks)
Additional context used
Ruff
fiftyone/core/frame.py

814-818: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

fiftyone/core/fields.py

188-189: Use a single if statement instead of nested if statements

Combine if statements using and

(SIM102)


282-285: Use ternary operator before_kwargs = kwargs if mode in ("before", "both") else {} instead of if-else-block

Replace if-else-block with before_kwargs = kwargs if mode in ("before", "both") else {}

(SIM108)


287-290: Use ternary operator after_kwargs = kwargs if mode in ("after", "both") else {} instead of if-else-block

Replace if-else-block with after_kwargs = kwargs if mode in ("after", "both") else {}

(SIM108)

fiftyone/core/odm/mixins.py

121-123: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)


296-296: Use key in dict instead of key in dict.keys()

Remove .keys()

(SIM118)

tests/unittests/dataset_tests.py

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

Rewrite key_fcn as a def

(E731)


1584-1584: Found useless expression. Either assign it to a variable or remove it.

(B018)


1586-1586: Found useless expression. Either assign it to a variable or remove it.

(B018)


2758-2758: Found useless expression. Either assign it to a variable or remove it.

(B018)


2769-2769: Found useless expression. Either assign it to a variable or remove it.

(B018)


3049-3049: Found useless expression. Either assign it to a variable or remove it.

(B018)


3060-3060: Found useless expression. Either assign it to a variable or remove it.

(B018)

fiftyone/core/dataset.py

1254-1254: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)


3454-3454: Ambiguous variable name: l

(E741)


3456-3456: Use key in dict instead of key in dict.keys()

Remove .keys()

(SIM118)


3653-3656: Use ternary operator doc_cls = self._frame_doc_cls if is_frame_field else self._sample_doc_cls instead of if-else-block

Replace if-else-block with doc_cls = self._frame_doc_cls if is_frame_field else self._sample_doc_cls

(SIM108)


7281-7281: Do not use bare except

(E722)

fiftyone/core/collections.py

652-652: Do not use bare except

(E722)

Additional comments not posted (163)
fiftyone/core/odm/utils.py (3)

Line range hint 187-235:
Integration of read_only parameter looks good.

The create_field function correctly includes the read_only parameter in its signature and documentation. The parameter is properly added to the field_kwargs dictionary, enhancing the field configurability.


Line range hint 304-321:
Handling of read_only attribute is correct.

The get_field_kwargs function correctly includes the read_only attribute in the returned dictionary, ensuring that this property is preserved during field processing.


340-354: New function _parse_field_str is well-implemented.

The _parse_field_str function correctly parses string representations of fields, extracting the field type and associated parameters. This enhances the flexibility of field handling.

fiftyone/core/odm/dataset.py (2)

Line range hint 70-130:
Integration of read_only field in SampleFieldDocument is correct.

The read_only field is properly added to the class and integrated into the to_field and from_field methods, enhancing the management of sample fields.


622-622: Addition of last_modified_at field in DatasetDocument is correct.

The last_modified_at field is properly added to the class, enhancing the dataset's metadata capabilities by tracking modification timestamps.

fiftyone/core/frame.py (4)

306-311: Integration of _enforce_read_only parameter in add_frame is correct.

The _enforce_read_only parameter is properly added to the method, allowing for more granular control over field modifications.


Line range hint 648-670:
Integration of timestamp parameters in _make_dict is correct.

The created_at and last_modified_at parameters are properly added to the method, enhancing its ability to track frame metadata.


Line range hint 722-792:
Refactoring of _save_replacements is efficient and flexible.

The method is correctly refactored to handle filtered fields and leverage the superclass's functionality, enhancing efficiency and flexibility.

Tools
Ruff

814-818: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)


798-824: Implementation of _validate_frame enhances validation clarity.

The method simplifies the validation process by focusing on individual frames, improving clarity and maintainability.

Tools
Ruff

814-818: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

fiftyone/core/fields.py (12)

Line range hint 26-89: LGTM! The validate_constraints function handles the read_only parameter correctly.

The addition of the read_only parameter is well-integrated into the existing logic.


Line range hint 92-128: LGTM! The matches_constraints function correctly checks the read_only status.

The integration of the read_only parameter is consistent with the function's purpose.


Line range hint 132-194: LGTM! The validate_field function correctly enforces the read_only constraint.

The addition of the read_only parameter is well-handled.


203-218: LGTM! The get_field_metadata function correctly includes the read_only attribute.

The function retrieves metadata as expected.


224-309: LGTM! The filter_schema function correctly applies the read_only filter.

The integration of the read_only parameter enhances the function's flexibility.

Tools
Ruff

282-285: Use ternary operator before_kwargs = kwargs if mode in ("before", "both") else {} instead of if-else-block

Replace if-else-block with before_kwargs = kwargs if mode in ("before", "both") else {}

(SIM108)


287-290: Use ternary operator after_kwargs = kwargs if mode in ("after", "both") else {} instead of if-else-block

Replace if-else-block with after_kwargs = kwargs if mode in ("after", "both") else {}

(SIM108)


311-357: LGTM! The flatten_schema function correctly handles the read_only parameter.

The function performs as expected with the new parameter.


Line range hint 361-398: LGTM! The _flatten function correctly incorporates the read_only parameter.

The function uses the matches_constraints function effectively.

Tools
Ruff

282-285: Use ternary operator before_kwargs = kwargs if mode in ("before", "both") else {} instead of if-else-block

Replace if-else-block with before_kwargs = kwargs if mode in ("before", "both") else {}

(SIM108)


287-290: Use ternary operator after_kwargs = kwargs if mode in ("after", "both") else {} instead of if-else-block

Replace if-else-block with after_kwargs = kwargs if mode in ("after", "both") else {}

(SIM108)


Line range hint 409-530: LGTM! The Field class correctly integrates the read_only attribute.

The property provides clear access to the read_only status.


587-594: LGTM! The IntField class correctly includes the read_only attribute.

The integration is consistent with other field classes.


609-616: LGTM! The ObjectIdField class correctly includes the read_only attribute.

The integration is consistent with other field classes.


637-644: LGTM! The UUIDField class correctly includes the read_only attribute.

The integration is consistent with other field classes.


653-660: LGTM! The BooleanField class correctly includes the read_only attribute.

The integration is consistent with other field classes.

fiftyone/core/odm/mixins.py (10)

Line range hint 132-159: LGTM! The set_field function correctly enforces the read-only constraint.

The _enforce_read_only parameter is well-integrated.

Tools
Ruff

121-123: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)


Line range hint 177-226: LGTM! The get_field_schema function correctly integrates the read_only parameter.

The function performs as expected with the new parameter.


Line range hint 234-324: LGTM! The merge_field_schema function correctly handles the overwrite parameter.

The integration enhances the flexibility of schema merging.

Tools
Ruff

296-296: Use key in dict instead of key in dict.keys()

Remove .keys()

(SIM118)


Line range hint 338-393: LGTM! The add_field function correctly integrates the read_only parameter.

The function allows fields to be marked as read-only upon creation.


Line range hint 454-466: LGTM! The _create_field function correctly integrates the read_only parameter.

The function allows fields to be created with the read_only attribute.


Line range hint 1059-1191: LGTM! The _merge_field function correctly handles the overwrite parameter.

The integration enhances the flexibility of field merging.


1433-1439: LGTM! The _insert function correctly sets timestamps for created_at and last_modified_at.

The integration ensures accurate tracking of document creation and modification.


1447-1457: LGTM! The _update function correctly updates the last_modified_at timestamp.

The integration ensures accurate tracking of document modifications.


Line range hint 671-851: LGTM! The DatasetMixin class correctly handles read-only fields and timestamp updates.

The integration enhances the functionality of the class.


117-123: Use raise ... from None in exception handling.

To distinguish this from errors in exception handling, use raise ... from None.

- raise AttributeError(
-     "%s has no field '%s'" % (self._doc_name(), field_name)
- )
+ raise AttributeError(
+     "%s has no field '%s'" % (self._doc_name(), field_name)
+ ) from None

Likely invalid or redundant comment.

Tools
Ruff

121-123: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

fiftyone/core/view.py (10)

21-23: Import addition approved.

The addition of ViewField from fiftyone.core.expressions aligns with its purpose and is consistent with handling expressions in views.


200-202: Property addition approved.

The _frame_cls property enhances the class's capabilities to manage frame-related data by returning fofr.FrameView.


537-537: Method invocation approved.

The invocation of _make_sample within _iter_samples aligns with the intended functionality and improves clarity.


548-552: Method definition approved.

The _make_sample method replaces _make_sample_fcn, aligning with the intended functionality and improving clarity in naming.


554-558: Method definition approved.

The _make_frame method mirrors _make_sample, aligning with the intended functionality and providing a parallel structure for handling frames.


586-610: Method definition approved.

The _init_make_frame method encapsulates the logic for creating frame instances and includes robust error handling.


846-846: Query enhancement approved.

The use of F(id_field) enhances querying capabilities in the get_group method.


Line range hint 917-962: Schema retrieval enhancement approved.

The addition of read_only and mode parameters in get_field_schema enhances flexibility in schema retrieval.


Line range hint 968-1018: Frame schema retrieval enhancement approved.

The addition of read_only and mode parameters in get_frame_field_schema enhances flexibility in frame schema retrieval.


1850-1865: Method addition approved.

The _get_edited_fields method provides an effective mechanism to track changes across view stages.

fiftyone/core/utils.py (3)

2156-2156: Initialization of _new_kwargs is appropriate.

The initialization of _new_kwargs in the __init__ method is correct and prepares the context manager for tracking new attributes.


2160-2165: Logic for tracking new attributes is sound.

The use of _new_kwargs to track new attributes separately from existing ones is correctly implemented.


2175-2176: Cleanup of new attributes is appropriate.

The removal of new attributes using _new_kwargs in the __exit__ method ensures that temporary changes do not persist.

fiftyone/utils/data/importers.py (5)

8-8: Import statement for datetime is appropriate.

The addition of datetime is necessary for handling timestamps in the code.


403-403: Method name change improves clarity.

Renaming _apply_field_schema to _apply_sample_field_schema clarifies the method's purpose.


1052-1053: Return type description update is clear.

The updated description provides clearer information about the return type of get_sample_field_schema.


1952-1953: Addition of timestamp fields for frames is appropriate.

The created_at and last_modified_at fields ensure accurate timestamping for imported frames.

Verify that these fields are correctly populated and used throughout the codebase.


Line range hint 1907-1924: Addition of timestamp fields is appropriate.

The created_at and last_modified_at fields ensure accurate timestamping for imported samples.

Verify that these fields are correctly populated and used throughout the codebase.

Verification successful

Verification of Timestamp Fields Usage

The created_at and last_modified_at fields are extensively used across the codebase, particularly in unit tests and various modules. This indicates that the fields are well-integrated and consistently utilized for timestamping purposes.

  • Files with Usage:
    • tests/unittests/patches_tests.py
    • tests/unittests/view_tests.py
    • tests/unittests/index_tests.py
    • tests/unittests/group_tests.py
    • tests/unittests/import_export_tests.py
    • tests/unittests/video_tests.py
    • tests/unittests/dataset_tests.py
    • fiftyone/utils/data/importers.py
    • fiftyone/core/dataset.py
    • fiftyone/core/odm/sample.py
    • fiftyone/core/odm/frame.py

The presence of these fields in tests and various modules confirms their correct implementation and usage.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that `created_at` and `last_modified_at` fields are used correctly.

# Test: Search for the usage of `created_at` and `last_modified_at`. Expect: Consistent usage across the codebase.
rg --type python -A 5 $'created_at|last_modified_at'

Length of output: 84


Script:

#!/bin/bash
# Description: Verify that `created_at` and `last_modified_at` fields are used correctly.

# Test: Search for the usage of `created_at` and `last_modified_at`. Expect: Consistent usage across the codebase.
rg --type py -A 5 $'created_at|last_modified_at'

Length of output: 193878

docs/source/user_guide/using_datasets.rst (6)

992-1006: Addition of created_at and last_modified_at fields is appropriate.

The created_at and last_modified_at fields are correctly added as read-only fields, automatically populated, which aligns with the PR objectives.


1025-1026: Example correctly demonstrates new fields.

The inclusion of created_at and last_modified_at fields in the example aligns with the updated default sample fields.


Line range hint 1074-1100: Field schema updates are consistent.

The field schema output correctly includes created_at and last_modified_at fields, reflecting the documentation changes.


Line range hint 1223-1251: Schema output correctly includes new fields.

The dataset field schema output appropriately includes created_at and last_modified_at, aligning with the documentation updates.


1439-1506: Metadata storage documentation is a valuable addition.

The section on storing field metadata, including descriptions, enhances the documentation by providing users with useful information on managing field metadata.


1508-1604: Read-only fields section is well-explained.

The documentation on read-only fields, including created_at and last_modified_at, effectively communicates their constraints and management, which is crucial for users.

tests/unittests/dataset_tests.py (2)

5120-5174: LGTM! The synchronization tests are comprehensive.

The test_sync_last_modified_at function effectively verifies the synchronization of the last_modified_at field.


5335-5370: LGTM! The test for new fields is well-implemented.

The test_set_new_fields function accurately tests the addition of new fields and the behavior of dynamic nested fields.

fiftyone/core/dataset.py (73)

34-34: LGTM!

The import of ViewField as F is consistent with its usage in the file.


Line range hint 60-65: LGTM!

The DatasetNotFoundError class is well-defined and inherits from ValueError appropriately.


374-376: LGTM!

The __delitem__ function correctly delegates to delete_samples.


689-692: LGTM!

The last_modified_at property is correctly implemented to return the dataset's last modified timestamp.


1281-1284: LGTM!

The get_field_schema method correctly includes the read_only parameter for handling read-only fields.


1325-1328: LGTM!

The get_frame_field_schema method correctly includes the read_only parameter for handling read-only fields.


1379-1382: LGTM!

The add_sample_field method correctly includes the read_only parameter for marking fields as read-only.


1525-1528: LGTM!

The add_frame_field method correctly includes the read_only parameter for marking fields as read-only.


1673-1676: LGTM!

The add_group_field method correctly includes the read_only parameter for marking fields as read-only.


2099-2104: LGTM!

The _remove_dynamic_sample_fields method is correctly implemented to purge and reload sample fields.


2130-2135: LGTM!

The _remove_dynamic_frame_fields method is correctly implemented to purge and reload frame fields.


Line range hint 2376-2380: LGTM!

The _iter_samples method is correctly implemented to iterate over samples and yield them.


Line range hint 2503-2508: LGTM!

The _iter_groups method is correctly implemented to iterate over groups and yield samples.


2832-2838: LGTM!

The _make_dict method correctly handles timestamps and converts samples to MongoDB dictionaries.


2832-2838: LGTM!

The _bulk_write method is correctly implemented for handling bulk writing operations.


3348-3351: LGTM!

The _is_read_only_field method is correctly implemented to check if a field is read-only.


9543-9547: LGTM!

The _set_field_read_only method is correctly implemented to set the read-only status of fields recursively.


8335-8363: LGTM!

The _clone_extras method is correctly implemented to clone various dataset components.


7859-7878: LGTM!

The _clone_collection method is correctly implemented for cloning dataset collections with timestamp handling.


7938-7947: LGTM!

The _get_samples_pipeline method is correctly implemented to construct a pipeline for sample operations.


7953-7962: LGTM!

The _get_frames_pipeline method is correctly implemented to construct a pipeline for frame operations.


8046-8059: LGTM!

The _get_edited_fields method is correctly implemented to retrieve edited fields from the dataset.


7167-7170: LGTM!

The _apply_sample_field_schema method is correctly implemented to apply a schema to sample fields.


7172-7175: LGTM!

The _apply_frame_field_schema method is correctly implemented to apply a schema to frame fields.


7274-7277: LGTM!

The _make_sample method is correctly implemented to convert a dictionary to a sample object.


7288-7290: LGTM!

The _make_frame method is correctly implemented to convert a dictionary to a frame object.


9493-9496: LGTM!

The _discard_none method is correctly implemented to remove None values from a list.


9497-9500: LGTM!

The _parse_fields method is correctly implemented to parse field names into a list.


9548-9551: LGTM!

The _extract_archive_if_necessary method is correctly implemented to handle archive extraction.


8335-8345: LGTM!

The _clone_reference_doc method is correctly implemented to clone reference documents.


8363-8366: LGTM!

The _clone_run method is correctly implemented to clone run documents.


9492-9493: LGTM!

The _get_random_characters method is correctly implemented to generate random characters.


7513-7514: LGTM!

The _validate_dataset_name method is correctly implemented to validate dataset names.


7513-7514: LGTM!

The _make_sample_collection_name method is correctly implemented to generate a sample collection name.


4135-4136: LGTM!

The _get_workspace_doc method is correctly implemented to retrieve workspace documents.


3890-3891: LGTM!

The _get_saved_view_doc method is correctly implemented to retrieve saved view documents.


3890-3891: LGTM!

The _load_saved_view_from_doc method is correctly implemented to load saved views from documents.


7440-7441: LGTM!

The _update_last_loaded_at method is correctly implemented to update the last loaded timestamp.


2135-2136: LGTM!

The _reload_docs method is correctly implemented to reload documents.


2104-2105: LGTM!

The _purge_fields method is correctly implemented to purge fields from a collection.


1361-1362: LGTM!

The _has_frame_fields method is correctly implemented to check for frame fields in a dataset.


1312-1318: LGTM!

The _get_field_schema method is correctly implemented to retrieve the field schema for a dataset.


1325-1331: LGTM!

The _get_frame_field_schema method is correctly implemented to retrieve the frame field schema for a dataset.


3359-3360: LGTM!

The _get_label_field_path method is correctly implemented to retrieve the path for a label field.


3361-3362: LGTM!

The _handle_frame_field method is correctly implemented to process frame fields.


3361-3362: LGTM!

The _get_label_field_root method is correctly implemented to retrieve the root for a label field.


7938-7947: LGTM!

The _get_samples_pipeline method is correctly implemented to construct a pipeline for sample operations.


7953-7962: LGTM!

The _get_frames_pipeline method is correctly implemented to construct a pipeline for frame operations.


8046-8059: LGTM!

The _get_edited_fields method is correctly implemented to retrieve edited fields from the dataset.


7167-7170: LGTM!

The _apply_sample_field_schema method is correctly implemented to apply a schema to sample fields.


7172-7175: LGTM!

The _apply_frame_field_schema method is correctly implemented to apply a schema to frame fields.


7274-7277: LGTM!

The _make_sample method is correctly implemented to convert a dictionary to a sample object.


7288-7290: LGTM!

The _make_frame method is correctly implemented to convert a dictionary to a frame object.


9493-9496: LGTM!

The _discard_none method is correctly implemented to remove None values from a list.


9497-9500: LGTM!

The _parse_fields method is correctly implemented to parse field names into a list.


9548-9551: LGTM!

The _extract_archive_if_necessary method is correctly implemented to handle archive extraction.


8335-8345: LGTM!

The _clone_reference_doc method is correctly implemented to clone reference documents.


8363-8366: LGTM!

The _clone_run method is correctly implemented to clone run documents.


9492-9493: LGTM!

The _get_random_characters method is correctly implemented to generate random characters.


7513-7514: LGTM!

The _validate_dataset_name method is correctly implemented to validate dataset names.


7513-7514: LGTM!

The _make_sample_collection_name method is correctly implemented to generate a sample collection name.


4135-4136: LGTM!

The _get_workspace_doc method is correctly implemented to retrieve workspace documents.


3890-3891: LGTM!

The _get_saved_view_doc method is correctly implemented to retrieve saved view documents.


3890-3891: LGTM!

The _load_saved_view_from_doc method is correctly implemented to load saved views from documents.


7440-7441: LGTM!

The _update_last_loaded_at method is correctly implemented to update the last loaded timestamp.


2135-2136: LGTM!

The _reload_docs method is correctly implemented to reload documents.


2104-2105: LGTM!

The _purge_fields method is correctly implemented to purge fields from a collection.


1361-1362: LGTM!

The _has_frame_fields method is correctly implemented to check for frame fields in a dataset.


1312-1318: LGTM!

The _get_field_schema method is correctly implemented to retrieve the field schema for a dataset.


1325-1331: LGTM!

The _get_frame_field_schema method is correctly implemented to retrieve the frame field schema for a dataset.


3359-3360: LGTM!

The _get_label_field_path method is correctly implemented to retrieve the path for a label field.


3361-3362: LGTM!

The _handle_frame_field method is correctly implemented to process frame fields.


3361-3362: LGTM!

The _get_label_field_root method is correctly implemented to retrieve the root for a label field.

fiftyone/core/collections.py (33)

11-11: Import addition is appropriate.

The addition of datetime is consistent with the need for timestamp handling in the file.


1233-1235: Method addition is appropriate.

The _is_read_only_field method correctly determines if a field is read-only using _parse_field.


1275-1276: Docstring update is accurate.

The addition of the read_only parameter in the docstring is clear and informative.


1287-1290: Constraints validation is well-integrated.

The read_only parameter is correctly included in the constraints validation, enhancing the method's robustness.


1311-1311: Return statement update is consistent.

The inclusion of read_only in the return values aligns with the method's purpose.


1333-1334: Initialization is necessary.

The initialization of read_only to None is necessary for its subsequent assignment.


1346-1346: Assignment is logical.

The assignment of read_only based on the field attribute ensures correct status capture.


1362-1362: Return statement update is consistent.

Including read_only in the return values aligns with the method's purpose.


1386-1393: Docstring update is accurate.

The addition of read_only and mode parameters in the docstring is clear and informative.


1422-1431: Docstring update is accurate.

The addition of read_only and mode parameters in the docstring is clear and informative.


1455-1456: Return statement update is consistent.

The update aligns with the method's purpose of returning field paths and instances.


1476-1478: Return statement update is consistent.

The update aligns with the method's purpose of returning field paths and instances.


1809-1810: Read-only check is necessary.

The check prevents unauthorized modifications to the tags field.


1854-1857: Read-only checks are necessary.

These checks ensure that modifications are not made to protected fields.


1914-1917: Read-only checks are necessary.

These checks ensure that modifications are not made to protected fields.


1939-1940: Read-only check is necessary.

The check prevents unauthorized modifications to label fields.


1953-1953: Timestamp update is consistent.

Updating last_modified_at during label modifications ensures accurate tracking.


1979-1979: Timestamp update is consistent.

Updating last_modified_at during label modifications ensures accurate tracking.


2408-2409: Read-only check is necessary.

The check prevents unauthorized modifications to fields.


2567-2568: Read-only check is necessary.

The check prevents unauthorized modifications to fields.


2919-2923: Read-only check and timestamp update are necessary.

The check prevents unauthorized modifications, and updating last_modified_at ensures accurate tracking.


2937-2941: Timestamp update is consistent.

Updating last_modified_at during modifications ensures accurate tracking.


2962-2966: Read-only check and timestamp update are necessary.

The check prevents unauthorized modifications, and updating last_modified_at ensures accurate tracking.


3003-3003: Timestamp update is consistent.

Updating last_modified_at during modifications ensures accurate tracking.


3024-3028: Read-only check and timestamp update are necessary.

The check prevents unauthorized modifications, and updating last_modified_at ensures accurate tracking.


3049-3049: Timestamp update is consistent.

Updating last_modified_at during modifications ensures accurate tracking.


3060-3070: Read-only check and timestamp update are necessary.

The check prevents unauthorized modifications, and updating last_modified_at ensures accurate tracking.


3095-3095: Timestamp update is consistent.

Updating last_modified_at during modifications ensures accurate tracking.


3109-3109: Timestamp update is consistent.

Updating last_modified_at during modifications ensures accurate tracking.


9442-9458: Index update is consistent.

Including created_at and last_modified_at in the default indexes optimizes queries involving timestamps.


9468-9499: Index update is consistent.

Including created_at and last_modified_at in the default indexes optimizes queries involving timestamps.


11057-11058: Timestamp update is consistent.

Updating last_modified_at in _parse_frame_values_dicts ensures accurate tracking of modifications.


11082-11087: Timestamp update is consistent.

Updating last_modified_at for new frame numbers ensures accurate tracking of modifications.

fiftyone/core/frame.py Show resolved Hide resolved
fiftyone/core/collections.py Show resolved Hide resolved
fiftyone/core/dataset.py Show resolved Hide resolved
fiftyone/core/dataset.py Show resolved Hide resolved
@@ -1785,6 +1825,16 @@ def only_matches(self):
"""Whether to only include samples that match the filter."""
return self._only_matches

def get_edited_fields(self, sample_collection, frames=False):
Copy link
Contributor

Choose a reason for hiding this comment

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

Some of these could share use a common mixin/util call, maybe?

Copy link
Contributor Author

@brimoor brimoor Aug 22, 2024

Choose a reason for hiding this comment

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

Yeah there is code duplication here, but it's also true for some of the other ViewStage interface methods so I was just following along with the existing broken windows 🙈

Copy link
Contributor

@benjaminpkane benjaminpkane left a comment

Choose a reason for hiding this comment

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

Nuts and bolts LGTM. I like the new mode parameter 🚀

@brimoor brimoor changed the title Adding created_at, last_modified_at, and read-only fields [HOLD] Adding created_at, last_modified_at, and read-only fields Aug 20, 2024
@swheaton
Copy link
Contributor

test failures 👀

@swheaton
Copy link
Contributor

forgot that PRs into other branches don't run the tests 🙃

@brimoor brimoor changed the title [HOLD] Adding created_at, last_modified_at, and read-only fields Adding created_at, last_modified_at, and read-only fields Sep 12, 2024
Copy link
Contributor

@swheaton swheaton left a comment

Choose a reason for hiding this comment

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

lgtm - @brimoor are we ready to merge into develop?

@brimoor
Copy link
Contributor Author

brimoor commented Sep 13, 2024

@swheaton I've still been pondering in background mode whether we should ship it like this or automatically update Dataset.last_modified_at/Sample.last_modified_at when samples/frames are updated, respectively. Any thoughts about that?

One material consequence is that we'd delete sync_last_modified_at() from the public interface as it would be a no-op.

Side note: I'm looking at #4765 now.

@swheaton
Copy link
Contributor

hmm ok will think about it.
4765 is still being worked on, although just some minor changes

@brimoor
Copy link
Contributor Author

brimoor commented Sep 15, 2024

Alright, code freeze approaches, this is fine as-is 🚀

@brimoor brimoor merged commit e3c144b into develop Sep 15, 2024
14 checks passed
@brimoor brimoor deleted the last-modified-at branch September 15, 2024 15:53
@swheaton
Copy link
Contributor

great thanks :shipit:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Work on a feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants