[Docs] Query performance sidebar #5167
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThe changes in this pull request primarily focus on the Changes
Possibly related PRs
Suggested reviewers
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? 🪧 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
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
docs/source/user_guide/app.rst (4)
410-411: Fix typos and redundant wordsThere are two minor text issues to fix:
- "comphrensive" should be "comprehensive"
- Remove the duplicate "the" in "the the"
-widgets are shown that include comphrensive counts. Granular filters can be -toggled for a dataset via the settings "Gear" or the the +widgets are shown that include comprehensive counts. Granular filters can be +toggled for a dataset via the settings "Gear" or the
403-407: Consider adding a note about index creation timingThe section explains index usage well but could benefit from emphasizing that creating indexes before adding samples is more efficient.
Consider adding a note like:
+.. note:: + + For optimal performance, it's recommended to create indexes before adding samples + to your dataset. This avoids the need to rebuild indexes after data insertion.
587-588: Enhance persistence behavior documentationWhile the note explains that changes persist between sessions, it would be helpful to clarify where this data is stored and any size limitations.
Consider expanding the note to include storage details:
- Any changes you make to a dataset's sidebar groups in the App are saved on - the dataset and will persist between sessions. + Any changes you make to a dataset's sidebar groups in the App are saved in + the dataset's metadata in the database and will persist between sessions. + These changes are also synchronized across different processes accessing + the same dataset.
627-630: Add performance consideration noteFor large datasets with many fields, it would be helpful to mention any performance implications of dynamic field assignment.
Consider adding:
property, these fields will be dynamically assigned to default groups in - the App at runtime. + the App at runtime. For datasets with a large number of fields, consider + explicitly defining sidebar groups to optimize the App's initialization time.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (5)
docs/source/images/app/app-field-tooltips.gifis excluded by!**/*.gif,!**/*.gifdocs/source/images/app/app-fields.gifis excluded by!**/*.gif,!**/*.gifdocs/source/images/app/app-filters.gifis excluded by!**/*.gif,!**/*.gifdocs/source/images/app/app-query-performance-mode.gifis excluded by!**/*.gif,!**/*.gifdocs/source/images/app/app-sidebar-mode.gifis excluded by!**/*.gif,!**/*.gif
📒 Files selected for processing (5)
app/packages/core/src/components/FieldLabelAndInfo/index.tsx(0 hunks)docs/source/release-notes.rst(1 hunks)docs/source/user_guide/app.rst(4 hunks)docs/source/user_guide/config.rst(1 hunks)docs/source/user_guide/using_datasets.rst(2 hunks)
💤 Files with no reviewable changes (1)
- app/packages/core/src/components/FieldLabelAndInfo/index.tsx
🔇 Additional comments (6)
docs/source/user_guide/config.rst (3)
672-674: LGTM: Clear documentation for default query performance setting.
The documentation clearly explains the default query performance configuration option and appropriately references the detailed section.
675-677: LGTM: Well-documented frame filtering configuration.
The documentation effectively explains the frame filtering disable option and includes a helpful reference to the detailed section.
678-680: LGTM: Clear documentation for query performance toggle.
The documentation clearly explains the query performance toggle configuration and includes an appropriate reference to the detailed section.
docs/source/user_guide/using_datasets.rst (2)
1648-1650: LGTM! Clear performance guidance added.
The added text provides valuable guidance about query performance limitations when working with frame-level fields.
1751-1753: LGTM! Clear explanation of summary field indexing.
The added text effectively explains how summary fields are indexed by default and can be leveraged by the App for performant filtering.
docs/source/release-notes.rst (1)
2228-2230: LGTM! Documentation update clearly explains the performance optimization.
The added text effectively communicates that sidebar filters can leverage indexes for improved performance, and includes a helpful reference link for more details.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
docs/source/user_guide/app.rst (2)
403-407: Consider adding a note about index creation timingWhile the documentation clearly explains how filters use indexes, it would be helpful to add a note that creating indexes after adding data to a collection can be time-consuming for large datasets. This could help users make informed decisions about when to create indexes.
By default, the sidebar filters are optimized to use indexes when no view is present. Filters that do have an index are highlighted with the lightning bolt icon. Query performance can be disabled by default via `default_query_performance` in your :ref:`App config <configuring-fiftyone-app>`. + +.. note:: + + Creating indexes after adding data to a collection can be time-consuming + for large datasets. For optimal performance, consider declaring indexes + before adding samples to your dataset.
569-570: Consider adding performance considerationsWhile the documentation clearly explains how to customize sidebar groups, it would be helpful to add a note about performance implications when working with large numbers of fields and groups. This could help users make informed decisions about group organization.
You can customize the layout of the App's sidebar by creating/renaming/deleting groups and dragging fields between groups directly in the App: .. note:: Any changes you make to a dataset's sidebar groups in the App are saved on the dataset and will persist between sessions. + + For datasets with a large number of fields, consider organizing fields + into a moderate number of groups to maintain optimal sidebar performance + and usability.Also applies to: 585-588
47b026a
What changes are proposed in this pull request?
Query performance will be the default sidebar mode going forward. This PR updates the main sidebar documentation and imagery to reflect that
Leveraging indexes while filteringandQuery Performance modesections into a singleOptimizing query performancesectionSidebar modesectionWhat areas of FiftyOne does this PR affect?
fiftyonePython library changesSummary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes