Minor fix#291
Merged
Merged
Conversation
…ebook update handlers
…sibleList components
…ows with informative messages
…ebookView for consistency
… adjust target_column_index retrieval
fix: refactor DashAIDataset creation for imbalanced converter
Feat/ux dataset
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR implements UX improvements including item editing functionality and fixes for supervised converters. The changes add inline editing for datasets and notebooks, improve the data handling for converter scopes and targets, and enhance visual consistency across the application.
Key changes:
- Added inline editing functionality for datasets and notebooks with keyboard and mouse interaction handling
- Fixed supervised converter parameter handling by updating target and scope data structures
- Enhanced UI consistency by updating colors, sizing, and adding informational note boxes
Reviewed Changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| setup.py | Version bump from 0.2.3 to 0.2.4 |
| DashAI/front/src/pages/datasets/Datasets.jsx | Added edit handlers for datasets and notebooks with API integration |
| DashAI/front/src/components/threeSectionLayout/ItemMenu.jsx | Added edit menu option with icon and handler |
| DashAI/front/src/components/threeSectionLayout/ItemBox.jsx | Implemented inline editing with TextField and keyboard/blur event handling |
| DashAI/front/src/components/threeSectionLayout/CollapsibleList.jsx | Added edit prop passing and updated icon colors |
| DashAI/front/src/components/notebooks/utils.js | Fixed column name extraction for scope formatting |
| DashAI/front/src/components/notebooks/notebookCreation/UploadNotebookSteps.jsx | Added informational note box |
| DashAI/front/src/components/notebooks/notebookCreation/CreateNotebookModal.jsx | Added informational note box |
| DashAI/front/src/components/notebooks/notebook/NotebookView.jsx | Fixed row item height |
| DashAI/front/src/components/notebooks/notebook/DatasetPreviewNotebook.jsx | Updated button styling and text |
| DashAI/front/src/components/notebooks/explorer/visualizations/TabularVisualizer.jsx | Updated pagination settings and removed scrollbar hiding |
| DashAI/front/src/components/notebooks/explorer/ExplorerBox.jsx | Updated styling to use theme colors |
| DashAI/front/src/components/notebooks/datasetCreation/SaveDatasetModal.jsx | Added informational note box |
| DashAI/front/src/components/notebooks/dataset/DatasetVisualization.jsx | Increased initial page size |
| DashAI/front/src/components/notebooks/dataset/DatasetTable.jsx | Added initial state for pagination model |
| DashAI/front/src/components/notebooks/converterCreation/ScopeStepConverter.jsx | Fixed column and target data structure handling |
| DashAI/front/src/components/notebooks/converterCreation/FormConverterSection.jsx | Updated target parameter name |
| DashAI/front/src/components/notebooks/converterCreation/ConverterTargetColumnModal.jsx | Updated target column selection logic |
| DashAI/front/src/components/notebooks/converter/ConverterBox.jsx | Updated parameter display and styling |
| DashAI/front/src/components/notebooks/NoteBox.jsx | New component for informational messages |
| DashAI/front/src/components/notebooks/LeftBar.jsx | Added edit handlers and improved item descriptions |
| DashAI/front/src/api/notebook.ts | Added updateNotebook API function |
| DashAI/back/job/dataset_job.py | Updated dataset creation with UUID-based folder names |
| DashAI/back/job/converter_job.py | Fixed target column handling for supervised converters |
| DashAI/back/converters/imbalanced_learn_wrapper.py | Simplified dataset creation by removing features parameter |
| DashAI/back/api/api_v1/schemas/notebook_params.py | Added NotebookUpdateParams schema |
| DashAI/back/api/api_v1/schemas/datasets_params.py | Removed columns field from DatasetUpdateParams |
| DashAI/back/api/api_v1/schemas/converter_params.py | Updated converter parameters schema for new data structures |
| DashAI/back/api/api_v1/endpoints/notebook.py | Added update_notebook endpoint |
| DashAI/back/api/api_v1/endpoints/datasets.py | Simplified dataset update logic |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if (e.key === "Enter") { | ||
| setIsEditing(false); | ||
| if (editedName.trim() !== name && editedName.trim() !== "") { | ||
| onEdit(editedName); |
There was a problem hiding this comment.
The onEdit callback should be called with the trimmed name, but it's being called with the untrimmed editedName. This could result in saving names with leading/trailing whitespace.
Suggested change
| onEdit(editedName); | |
| onEdit(editedName.trim()); |
Comment on lines
+33
to
+35
| noRowsOverlay: { | ||
| style: { height: "100%" }, | ||
| }, |
There was a problem hiding this comment.
The noRowsOverlay property should be placed inside the slotProps object, not as a top-level property in the DataGrid configuration.
| dataset_path = f"{converter_list.notebook.file_path}/dataset" | ||
| loaded_dataset = load_dataset(dataset_path) | ||
| target_column_index = converter_list.parameters.pop("target_index") | ||
| print("Pre target column") |
There was a problem hiding this comment.
Debug print statements should be removed from production code. Use proper logging with the existing log object instead.
Suggested change
| print("Pre target column") | |
| log.debug("Pre target column") |
Comment on lines
+214
to
+221
| print("Pre target column") | ||
| params = converter_list.parameters or {} | ||
| target_column_index = ( | ||
| params["target"].get("idx") | ||
| if params.get("target") is not None | ||
| else None | ||
| ) | ||
| print(target_column_index) |
There was a problem hiding this comment.
Debug print statements should be removed from production code. Use proper logging with the existing log object instead.
Suggested change
| print("Pre target column") | |
| params = converter_list.parameters or {} | |
| target_column_index = ( | |
| params["target"].get("idx") | |
| if params.get("target") is not None | |
| else None | |
| ) | |
| print(target_column_index) | |
| log.debug("Pre target column") | |
| params = converter_list.parameters or {} | |
| target_column_index = ( | |
| params["target"].get("idx") | |
| if params.get("target") is not None | |
| else None | |
| ) | |
| log.debug(f"Target column index: {target_column_index}") |
| ) | ||
| if not params.name or params.name == notebook.name: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_304_NOT_MODIFIED, |
There was a problem hiding this comment.
HTTP 304 Not Modified is semantically incorrect here as it's for conditional requests. Use HTTP 400 Bad Request or return the existing notebook with HTTP 200 instead.
Suggested change
| status_code=status.HTTP_304_NOT_MODIFIED, | |
| status_code=status.HTTP_400_BAD_REQUEST, |
| id: number, | ||
| formData: object, | ||
| ): Promise<INotebook> => { | ||
| console.log("updating notebook with id:", id, "and formData:", formData); |
There was a problem hiding this comment.
Console.log statements should be removed from production code as they can leak sensitive information and clutter the console.
Suggested change
| console.log("updating notebook with id:", id, "and formData:", formData); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Added some UX changes and fixed supervised converters