Merge release/v1.4.0 to develop
#5569
Conversation
frontend teams -> enterprise
* ensure read_only is accessed for fo fields only * safe field.read_only * another one * re-fix, we cannot update metadata for non-FO Field * more field.read_only protections * Revert "more field.read_only protections" This reverts commit a51251d. * read_only protection that is necessary in obscure case --------- Co-authored-by: brimoor <[email protected]> Co-authored-by: Stuart Wheaton <[email protected]>
Fiftyone->FiftyOne rename teams docs folders rename teams files teams->enterprise add redirects from teams files to now enterprise fix nav/footer layout
Fix YOLO kwargs syntax
…t-so-2 [DOCS] Update docs for FiftyOne Enterprise
* optimize qp counts with count scans * optional fixes * update gql output
WalkthroughThe changes introduce a new query performance feature across several components. A new property is added to the aggregation query in the state package and mirrored in the GraphQL schema via the AggregationForm input type. Additionally, both core and server aggregation logic have been updated to support optional filtering based on optimization flags (_optimize and query_performance). The test suite is extended to cover scenarios where query performance is toggled. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant GraphQL
participant AggregationHandler
participant AggregationLogic
Client->>GraphQL: Send Aggregation Request (queryPerformance flag)
GraphQL->>AggregationHandler: Map input to AggregationForm
AggregationHandler->>AggregationLogic: Invoke aggregation with _optimize/query_performance
AggregationLogic-->>AggregationHandler: Return aggregation result (conditional filtering)
AggregationHandler-->>GraphQL: Return processed results
GraphQL-->>Client: Respond with final aggregation output
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
fiftyone/core/aggregations.py (1)
588-594: Conditional filtering based on optimization flag.When the
_optimizeflag is set toTrue, the code now skips filtering out None values. This improves performance by removing a potentially expensive match stage from the aggregation pipeline.This code could be slightly simplified by combining the conditional checks.
- # todo: can we omit filtering out none in all situations - if not self._optimize: - if not sample_collection._contains_videos() or path != "frames": - pipeline.append( - {"$match": {"$expr": {"$gt": ["$" + path, None]}}} - ) + # todo: can we omit filtering out none in all situations + if not self._optimize and (not sample_collection._contains_videos() or path != "frames"): + pipeline.append( + {"$match": {"$expr": {"$gt": ["$" + path, None]}}} + )🧰 Tools
🪛 Ruff (0.8.2)
589-590: Use a single
ifstatement instead of nestedifstatements(SIM102)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
app/packages/relay/src/fragments/__generated__/estimatedCountsFragment.graphql.tsis excluded by!**/__generated__/**,!**/__generated__/**app/packages/relay/src/queries/__generated__/aggregationsQuery.graphql.tsis excluded by!**/__generated__/**,!**/__generated__/**
📒 Files selected for processing (5)
app/packages/state/src/recoil/aggregations.ts(2 hunks)app/schema.graphql(3 hunks)fiftyone/core/aggregations.py(6 hunks)fiftyone/server/aggregations.py(7 hunks)tests/unittests/server_aggregations_tests.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{ts,tsx}`: Review the Typescript and React code for co...
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.
app/packages/state/src/recoil/aggregations.ts
🪛 Ruff (0.8.2)
fiftyone/core/aggregations.py
589-590: Use a single if statement instead of nested if statements
(SIM102)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: test / test-python (ubuntu-latest-m, 3.11)
- GitHub Check: test / test-python (ubuntu-latest-m, 3.10)
- GitHub Check: test / test-python (ubuntu-latest-m, 3.9)
- GitHub Check: test / test-app
- GitHub Check: e2e / test-e2e
- GitHub Check: build / build
- GitHub Check: lint / eslint
- GitHub Check: build
🔇 Additional comments (19)
app/packages/state/src/recoil/aggregations.ts (2)
16-16: New import added for query performance support.This import brings in the queryPerformance state that will be used to control aggregation optimization behavior.
89-89: Added queryPerformance to aggregation form variables.The queryPerformance flag is now included in the GraphQL aggregation form, enabling the server to optimize query execution based on this flag. This works in conjunction with the schema changes that add this field to the AggregationForm input type.
app/schema.graphql (3)
35-35: Added queryPerformance option to AggregationForm input type.This boolean field (defaulting to false) allows clients to request optimized query processing. This change coordinates with the state management updates in the aggregations.ts file.
242-242: Changed estimatedSampleCount from non-nullable to nullable.The
estimatedSampleCountfield is now nullable (Intinstead ofInt!), allowing for cases where sample count estimation may not be available.
799-799: Made StringAggregation values field nullable.This change allows StringAggregation results to return without values when query performance is enabled. The field type has changed from
[StringAggregationValue!]!to[StringAggregationValue!], supporting the optimization where values can be omitted to improve performance.tests/unittests/server_aggregations_tests.py (2)
97-118: Refactored test to use an explicit form dictionary.The test has been restructured to define a separate
formdictionary that is then passed to theexecutefunction. This makes the code more maintainable and sets up for testing the query performance feature.
136-152: Added test case for query performance optimization.This new test case verifies that when
query_performanceis set toTrue, thevaluesfield in the response is set toNone. This confirms the new optimization behavior where value calculations are skipped to improve performance.fiftyone/core/aggregations.py (4)
540-550: Added _optimize parameter to Count class constructor.The Count aggregation now supports a new
_optimizeparameter that controls whether certain filtering operations are performed. This enables performance improvements when query performance is prioritized.
585-585: Passing optimize parameter to _parse_field_and_expr function.The newly added
_optimizeflag is now passed to the parsing function, allowing optimization behavior to propagate through the aggregation processing.
3007-3007: Added optimize parameter to _parse_field_and_expr function.The function now accepts an
optimizeparameter which controls whether certain pipeline stages are included, enabling performance optimizations.
3114-3115: Skip project stage when optimize is True.When the
optimizeflag is enabled and no context is present, the code now skips adding a project stage to the pipeline, further improving performance for optimized queries.fiftyone/server/aggregations.py (8)
45-45: Good addition of query_performance parameterThe new optional
query_performanceparameter with a default value ofFalseis well-structured and logically placed. This addition will allow for toggling query performance optimizations, which aligns with the PR's purpose of enhancing aggregation performance.
57-58: Improved initialization of boolean propertiesThe changes to initialize
falseandtrueproperties with a default value of0is a good practice. This prevents potential issues with uninitialized variables and makes the code more robust.
74-78: Proper initialization of float-related propertiesAdding default values of
0toinf,nan, andninfproperties is a good improvement. This ensures these properties have sensible initial values, which prevents potential issues later in the code execution.
96-96: Made values property properly optionalMaking the
valuesproperty optional with a default ofNoneis appropriate as these values might not always be present in the aggregation results. This change aligns with the actual behavior of the aggregation system.
145-148: Correctly passing query_performance parameterThe parameter is now properly passed to the
_resolve_path_aggregationfunction in the list comprehension. This ensures the performance optimization settings are respected throughout the aggregation process.
216-216: Updated function signatureThe function signature has been correctly updated to include the new
query_performanceparameter, maintaining consistency with the changes in the calling function.
219-221: Added optimization flag to Count aggregationThe
_optimizeparameter is now passed to theCountaggregation, controlled by thequery_performancevalue. This enables conditional optimization for counting operations, which can significantly improve performance for large datasets.
234-254: Well-structured conditional aggregation logicThe implementation of conditional aggregation logic based on the
query_performanceflag is well-designed. When performance optimization is not required (query_performance=False), additional aggregations are appended based on field type:
CountValuesfor boolean fieldsBoundsfor date, datetime, and integer fieldsBoundswith special parameters for float fieldsCountValueswith limit for string fieldsThis approach provides a good balance between rich data collection and performance optimization.
Merge
release/v1.4.0todevelopSummary by CodeRabbit
New Features
Tests