change type to array of object for view param of set_view #4198
Conversation
WalkthroughThe recent updates focus on enhancing the testing and user interaction aspects of the application. New attributes like Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release/v0.23.8 #4198 +/- ##
==================================================
Coverage ? 27.99%
==================================================
Files ? 814
Lines ? 101649
Branches ? 1178
==================================================
Hits ? 28456
Misses ? 73193
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- app/packages/operators/src/built-in-operators.ts (1 hunks)
Additional comments: 1
app/packages/operators/src/built-in-operators.ts (1)
- 473-473: The modification to the
SetViewclass'sresolveInputmethod, changing theviewparameter to accept an array of objects, aligns with the PR's objective to enhance data view flexibility. This change allows for more complex data view configurations, which can significantly improve user experience by enabling more nuanced data manipulation and visualization within the FiftyOne application.
imanjra
force-pushed
the
bugfix/set-view-array
branch
from
0d1ed14
to
91c1e38
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
e2e-pw/package.jsonis excluded by!**/*.jsone2e-pw/src/shared/assets/plugins/e2e/fiftyone.ymlis excluded by!**/*.yml
Files selected for processing (8)
- .github/workflows/e2e.yml (1 hunks)
- app/packages/core/src/components/ViewBar/ViewBar.tsx (1 hunks)
- app/packages/operators/src/OperatorBrowser.tsx (2 hunks)
- app/packages/operators/src/OperatorPalette.tsx (3 hunks)
- e2e-pw/src/oss/poms/operators/operators-browser.ts (1 hunks)
- e2e-pw/src/oss/poms/viewbar/viewbar.ts (1 hunks)
- e2e-pw/src/oss/specs/operators/built-in-operators.spec.ts (1 hunks)
- e2e-pw/src/shared/assets/plugins/e2e/init.py (1 hunks)
Files skipped from review due to trivial changes (1)
- app/packages/core/src/components/ViewBar/ViewBar.tsx
Additional Context Used
Additional comments not posted (10)
e2e-pw/src/shared/assets/plugins/e2e/__init__.py (2)
6-7: Theserialize_viewfunction serializes a view object to JSON. Ensure that the view object's_serializemethod is compatible with thejson_util.dumpsmethod for accurate serialization.
18-21: Theexecutemethod inE2ESetViewsets a limited view and triggers the "set_view" event. Given the PR's objective to handle an array of views, consider updating the logic to support multiple views if applicable.e2e-pw/src/oss/poms/viewbar/viewbar.ts (1)
3-26: TheViewBarPomclass provides methods for interacting with the view bar component. Ensure that the test IDs used (e.g., "view-bar", "btn-clear-view-bar", "view-stage-container") are correctly assigned in the corresponding HTML elements for these methods to work as expected.e2e-pw/src/oss/poms/operators/operators-browser.ts (1)
3-31: TheOperatorsBrowserPomclass provides methods for interacting with the operators browser component. Ensure that the test IDs used (e.g., "operators-browser", "action-browse-operations", "operators-browser-search") are correctly assigned in the corresponding HTML elements for these methods to work as expected.e2e-pw/src/oss/specs/operators/built-in-operators.spec.ts (1)
54-59: The test case "Built-in operators: set view" verifies the "set view" functionality. Ensure that the "E2E: Set view" operator and the "Limit3" view stage are correctly implemented and that this test aligns with the expected behavior after the PR changes..github/workflows/e2e.yml (1)
15-15: The addition of theFIFTYONE_PLUGINS_DIRenvironment variable is a good practice for specifying the directory for plugins. Ensure that the specified path (${{ github.workspace }}/src/shared/assets/plugins) correctly points to the intended directory for plugins.app/packages/operators/src/OperatorBrowser.tsx (2)
147-147: The addition of thedata-cy="operators-browser-search"attribute is a good practice for facilitating testing. Ensure that this attribute is used consistently in tests that interact with the search input field.
176-176: The addition of thedata-cy="operators-browser"attribute to thePaperPropsof the dialog component is a good practice for testing. Ensure that this attribute is used consistently in tests that interact with the operators browser dialog.app/packages/operators/src/OperatorPalette.tsx (2)
48-48: The introduction of thedialogPropsproperty in theOperatorPalettePropsinterface allows for more flexible configuration of theDialogcomponent. Ensure that this new property is documented and used consistently across the application.
91-101: The usage of thedialogPropsproperty to merge existingPaperPropswith a new style setting (backgroundImage: "none") is a good practice for enhancing component flexibility. Ensure that this approach does not inadvertently override important existing properties.
There was a problem hiding this comment.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
e2e-pw/src/shared/assets/plugins/e2e/fiftyone.ymlis excluded by!**/*.yml
Files selected for processing (8)
- .github/workflows/e2e.yml (1 hunks)
- app/packages/core/src/components/ViewBar/ViewBar.tsx (1 hunks)
- app/packages/operators/src/OperatorBrowser.tsx (2 hunks)
- app/packages/operators/src/OperatorPalette.tsx (3 hunks)
- e2e-pw/src/oss/poms/operators/operators-browser.ts (1 hunks)
- e2e-pw/src/oss/poms/viewbar/viewbar.ts (1 hunks)
- e2e-pw/src/oss/specs/operators/built-in-operators.spec.ts (1 hunks)
- e2e-pw/src/shared/assets/plugins/e2e/init.py (1 hunks)
Files skipped from review as they are similar to previous changes (8)
- .github/workflows/e2e.yml
- app/packages/core/src/components/ViewBar/ViewBar.tsx
- app/packages/operators/src/OperatorBrowser.tsx
- app/packages/operators/src/OperatorPalette.tsx
- e2e-pw/src/oss/poms/operators/operators-browser.ts
- e2e-pw/src/oss/poms/viewbar/viewbar.ts
- e2e-pw/src/oss/specs/operators/built-in-operators.spec.ts
- e2e-pw/src/shared/assets/plugins/e2e/init.py
Additional Context Used
imanjra
force-pushed
the
bugfix/set-view-array
branch
from
91c1e38
to
abbdd1c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
e2e-pw/src/shared/assets/plugins/e2e/fiftyone.ymlis excluded by!**/*.yml
Files selected for processing (8)
- .github/workflows/e2e.yml (1 hunks)
- app/packages/core/src/components/ViewBar/ViewBar.tsx (1 hunks)
- app/packages/operators/src/OperatorBrowser.tsx (2 hunks)
- app/packages/operators/src/OperatorPalette.tsx (3 hunks)
- e2e-pw/src/oss/poms/operators/operators-browser.ts (1 hunks)
- e2e-pw/src/oss/poms/viewbar/viewbar.ts (1 hunks)
- e2e-pw/src/oss/specs/operators/built-in-operators.spec.ts (1 hunks)
- e2e-pw/src/shared/assets/plugins/e2e/init.py (1 hunks)
Files skipped from review as they are similar to previous changes (8)
- .github/workflows/e2e.yml
- app/packages/core/src/components/ViewBar/ViewBar.tsx
- app/packages/operators/src/OperatorBrowser.tsx
- app/packages/operators/src/OperatorPalette.tsx
- e2e-pw/src/oss/poms/operators/operators-browser.ts
- e2e-pw/src/oss/poms/viewbar/viewbar.ts
- e2e-pw/src/oss/specs/operators/built-in-operators.spec.ts
- e2e-pw/src/shared/assets/plugins/e2e/init.py
Additional Context Used
imanjra
force-pushed
the
bugfix/set-view-array
branch
from
abbdd1c
to
5e98e03
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
e2e-pw/src/shared/assets/plugins/e2e/fiftyone.ymlis excluded by!**/*.yml
Files selected for processing (10)
- .github/workflows/e2e.yml (1 hunks)
- app/packages/core/src/components/ViewBar/ViewBar.tsx (1 hunks)
- app/packages/operators/src/OperatorBrowser.tsx (2 hunks)
- app/packages/operators/src/OperatorPalette.tsx (3 hunks)
- app/packages/operators/src/built-in-operators.ts (1 hunks)
- e2e-pw/src/oss/poms/operators/operators-browser.ts (1 hunks)
- e2e-pw/src/oss/poms/viewbar/viewbar.ts (1 hunks)
- e2e-pw/src/oss/specs/operators/built-in-operators.spec.ts (1 hunks)
- e2e-pw/src/shared/assets/plugins/e2e/init.py (1 hunks)
- setup.py (1 hunks)
Files skipped from review as they are similar to previous changes (9)
- .github/workflows/e2e.yml
- app/packages/core/src/components/ViewBar/ViewBar.tsx
- app/packages/operators/src/OperatorBrowser.tsx
- app/packages/operators/src/OperatorPalette.tsx
- app/packages/operators/src/built-in-operators.ts
- e2e-pw/src/oss/poms/operators/operators-browser.ts
- e2e-pw/src/oss/poms/viewbar/viewbar.ts
- e2e-pw/src/oss/specs/operators/built-in-operators.spec.ts
- e2e-pw/src/shared/assets/plugins/e2e/init.py
Additional comments not posted (1)
setup.py (1)
19-19: The version update from "0.23.7" to "0.23.8" is noted and appears correct. Ensure that this version change is reflected in all relevant documentation, release notes, and any other dependencies that might be affected by this update.
What changes are proposed in this pull request?
change type to array of object for view param of set_view
How is this patch tested? If it is not, please explain why.
Using a test operator
Release Notes
Is this a user-facing change that should be mentioned in the release notes?
notes for FiftyOne users.
change type to array of object for view param of set_view
What areas of FiftyOne does this PR affect?
fiftyonePython library changesSummary by CodeRabbit
New Features
data-cyattributes to various components in the app.fiftyoneoperators.Chores
FIFTYONE_PLUGINS_DIRin the workflow for plugins directory setup.