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

chore(UI): Remove view builder #1267

Merged
merged 11 commits into from
Feb 3, 2025
Merged

chore(UI): Remove view builder #1267

merged 11 commits into from
Feb 3, 2025

Conversation

rouk1
Copy link
Contributor

@rouk1 rouk1 commented Jan 30, 2025

Closes #1266.

This PR removes all view builder related code.
Activity feed item now have a version number in backend aswell as in frontend.

User can now annotate there items in the activity feed.

UI preview:

feed.mp4

Copy link
Contributor

github-actions bot commented Jan 30, 2025

Coverage Report for frontend

Status Category Percentage Covered / Total
🔵 Lines 81.82% 2193 / 2680
🔵 Statements 81.82% 2193 / 2680
🔵 Functions 50.56% 45 / 89
🔵 Branches 83.26% 194 / 233
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
skore-ui/src/SkoreUi.vue 0% 0% 0% 0% 1-49
skore-ui/src/StandaloneWidget.vue 0% 0% 0% 0% 1-21
skore-ui/src/dto.ts 100% 100% 100% 100%
skore-ui/src/models.ts 75.86% 20% 100% 75.86% 30-31, 33-34, 36-38
skore-ui/src/router.ts 83.87% 100% 0% 83.87% 33-37
skore-ui/src/components/FloatingTooltip.vue 100% 100% 100% 100%
skore-ui/src/components/RichTextEditor.vue 100% 100% 100% 100%
skore-ui/src/services/api.ts 83.05% 63.63% 100% 83.05% 18-19, 53-56, 64-67
skore-ui/src/views/AppToolbar.vue 0% 0% 0% 0% 1-54
skore-ui/src/views/activity/ActivityFeedCardHeader.vue 100% 100% 100% 100%
skore-ui/src/views/activity/ActivityFeedItem.vue 100% 100% 100% 100%
skore-ui/src/views/activity/ActivityFeedView.vue 96.96% 66.66% 100% 96.96% 14
skore-ui/src/views/activity/ItemNoteEditor.vue 100% 100% 100% 100%
skore-ui/src/views/activity/activity.ts 100% 90.9% 100% 100%
Generated in workflow #193 for commit 86feb8d by the Vitest Coverage Report Action

 remove skore/src/skore/persistence/repository/view_repository.py
Copy link
Contributor

github-actions bot commented Jan 30, 2025

Coverage

Coverage Report for backend
FileStmtsMissCoverMissing
venv/lib/python3.12/site-packages/skore
   __init__.py140100% 
   __main__.py880%3–19
   exceptions.py440%4–23
venv/lib/python3.12/site-packages/skore/cli
   __init__.py550%3–8
   cli.py22220%3–70
   color_format.py49490%3–116
venv/lib/python3.12/site-packages/skore/persistence
   __init__.py00100% 
venv/lib/python3.12/site-packages/skore/persistence/item
   __init__.py56393%96–99
   altair_chart_item.py24193%15
   item.py24196%86
   matplotlib_figure_item.py42196%19
   media_item.py240100% 
   numpy_array_item.py29194%16
   pandas_dataframe_item.py31194%14
   pandas_series_item.py31194%14
   pickle_item.py330100% 
   pillow_image_item.py32194%16
   plotly_figure_item.py25193%15
   polars_dataframe_item.py29194%14
   polars_series_item.py24193%14
   primitive_item.py25291%13–15
   sklearn_base_estimator_item.py31194%15
   skrub_table_report_item.py10186%11
venv/lib/python3.12/site-packages/skore/persistence/repository
   __init__.py20100% 
   item_repository.py59591%15–16, 202–203, 226
venv/lib/python3.12/site-packages/skore/persistence/storage
   __init__.py40100% 
   abstract_storage.py220100% 
   disk_cache_storage.py33195%44
   in_memory_storage.py200100% 
venv/lib/python3.12/site-packages/skore/persistence/view
   __init__.py220%3–5
   view.py550%3–19
venv/lib/python3.12/site-packages/skore/project
   __init__.py30100% 
   _launch.py150199%278
   _open.py90100% 
   project.py62199%236
venv/lib/python3.12/site-packages/skore/sklearn
   __init__.py40100% 
   _base.py140497%91, 94, 168–>173, 183–184
   find_ml_task.py45391%71–>87, 84–86
   types.py20100% 
venv/lib/python3.12/site-packages/skore/sklearn/_cross_validation
   __init__.py60100% 
   metrics_accessor.py1630100% 
   report.py870100% 
venv/lib/python3.12/site-packages/skore/sklearn/_estimator
   __init__.py60100% 
   metrics_accessor.py265497%149–158, 183–>236, 191, 434–>437, 783–>786
   report.py120099%213–>219, 221–>223
   utils.py11110%1–19
venv/lib/python3.12/site-packages/skore/sklearn/_plot
   __init__.py40100% 
   precision_recall_curve.py121198%229–>246, 317
   prediction_error.py970100% 
   roc_curve.py1280100% 
   utils.py880100% 
venv/lib/python3.12/site-packages/skore/sklearn/train_test_split
   __init__.py00100% 
   train_test_split.py36294%16–17
venv/lib/python3.12/site-packages/skore/sklearn/train_test_split/warning
   __init__.py80100% 
   high_class_imbalance_too_few_examples_warning.py17378%16–18, 80
   high_class_imbalance_warning.py18288%16–18
   random_state_unset_warning.py11187%15
   shuffle_true_warning.py9091%44–>exit
   stratify_is_set_warning.py11187%15
   time_based_column_warning.py22189%17, 69–>exit
   train_test_split_warning.py5180%21
venv/lib/python3.12/site-packages/skore/ui
   __init__.py00100% 
   app.py32320%3–83
   dependencies.py440%3–10
   project_routes.py40400%3–91
   server.py17170%3–40
venv/lib/python3.12/site-packages/skore/utils
   __init__.py60100% 
   _accessor.py70100% 
   _environment.py26097%29–>34
   _logger.py21484%14–18
   _patch.py11546%19–35
   _progress_bar.py300100% 
   _show_versions.py310100% 
TOTAL259225690% 

Tests Skipped Failures Errors Time
540 3 💤 0 ❌ 0 🔥 2m 21s ⏱️

Copy link
Contributor

github-actions bot commented Jan 30, 2025

Documentation preview @ 86feb8d

@rouk1 rouk1 changed the title chore (UI): Remove view builder chore(UI): Remove view builder Jan 30, 2025
@rouk1 rouk1 marked this pull request as ready for review January 31, 2025 12:23
@rouk1 rouk1 marked this pull request as draft January 31, 2025 12:27
@rouk1 rouk1 marked this pull request as ready for review January 31, 2025 12:40
Copy link
Contributor

@MarieS-WiMLDS MarieS-WiMLDS left a comment

Choose a reason for hiding this comment

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

I tried with the window covering half of my screen, and it's great!! I really like that there is little margin, in a context where I need all the space to be useful. I also like where the annotation pencil is now.
image

I noticed these three points, none of them is a big deal.

point 1

image

when hovering the project name, it gives me the location. It's nice so i'm not lost, but i'd like to have it complete. Is it possible to display it on several lines? or on the right?

point 2

I don't think that it's something introduced in the PR, but if it's not too difficult, when the item contains text, I'd like to have it in a grey rectangle like the other objects, otherwise it get mixed with the notes (cf below).

point 3

I like the idea to have a #n after the object name to display that there is another one! But it's very small, i had a hard time reading.

image

image

@rouk1
Copy link
Contributor Author

rouk1 commented Jan 31, 2025

Point 1

Tooltip will now never be larger than 50% of the viewport and placed at the bottom start of the related element.

Screenshot 2025-01-31 at 18 23 24

Point 2

Nice catch !
This would apply to all "markdown" items. So I'd like to propose something else, I've added a left border to the notes to use the same pattern as citation you can find pretty much everywhere. wdyt ? In the mean time @anasstarfaoui may have some better ideas : )

Screenshot 2025-01-31 at 18 30 37

Point 3

Ok it's now the same size as the name of the item.
Screenshot 2025-01-31 at 18 36 25

@rouk1 rouk1 requested a review from MarieS-WiMLDS January 31, 2025 17:37
@MarieS-WiMLDS
Copy link
Contributor

it's good for me from a functional point of view, thanks :) !
I'll let a dev give his opinion for the technical aspects.

@thomass-dev thomass-dev self-requested a review February 3, 2025 10:21
Copy link
Contributor

@auguste-probabl auguste-probabl left a comment

Choose a reason for hiding this comment

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

At a glance this looks fine to me

@thomass-dev thomass-dev merged commit b85b797 into probabl-ai:main Feb 3, 2025
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove the views section from the UI
4 participants