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

feat: eda plot view #1161

Merged
merged 88 commits into from
May 11, 2024
Merged

feat: eda plot view #1161

merged 88 commits into from
May 11, 2024

Conversation

SmiteDeluxe
Copy link
Contributor

@SmiteDeluxe SmiteDeluxe commented May 9, 2024

Closes #955
Closes #986

Summary of Changes

Implemented history/action structure for webview to add new actions to state and have them (if external and info not already present) send execute requests to runner that are then cancellable or deployed in correct order. Only fully working for Plots/Tabs at the moment, that are on deploy added to tabs state and set as currentIndex. All Tabs and Table retain their state. Runner uses existing methods in RunnerAPI to get back to relevant state by executing past manipulating actions and then returns the result of the new action (only if plots right now).

Tabs can be created by selecting columns and right clicking, zooming in on profiling images or by creating an empty tab with the plus icon in the sidebar. There in the guided menu users can change the current Tab to display other info. At that point the tab will go into building state and show prompts, loading screens and buttons accordingly. Typings are adapted to abstract as much as possible (mainly around column count for tabs, none, one and two) and stores are heavily used for reactivity.

- runnerApi that now handles any eda vscode requests to runner, for starter getPlaceholderValue
     - edaPanel now renamed panelsMap to instancesMap that also saves RunnerApi instance
     - RunnerApi instance takes: services, pipelinePath (which is needed to get the document of pipeline that is to be extended)
- along with above no longer option for PanelIdentifier or pipelineId to be undefined, not needed anymore without dev methods that start blank eda sessions and this way more safe and solid
- revaling already exisiting panel bug where undefined state, found out it's because the _update() method is only fully executed after current state was already found
     - new variable updateHtmlDone that is set to false on either creating new panel or revealing current one and to true on _update() done; constructCurrentState will only be executed once that variable is true or with a timeout of 10s
- filters:
    - for the filter to decide between: 1. search string 2. value range 3. distinct value, the tableView must decide for each column if numerical => value range or categorical => many values => search string OR little Values => distinct value
        - for that profiling for numerical items (that show % for example) now have new property "interpretation" which can be "warn" (for missing val), "category" (for this) or "default"
        - faster than iterating through categorical cols to count values for huge data, since we have this info on profiling generation already
    - whereas for value range find min max ourselves, since otherwise would need more in pipeline and more placeholder value queries, and this should be faster
filters then in own component that decides what to show and calls vscode to initiate the runner code execution
- as deliberated before kind of: selections are now not cleared by clicks anywhere anymore but only clicks on main cells, as the global window listener to clear was getting too convoluted, now don't need the rowClicked or columnClicked params anymore
    - preventClicks store is now set to false on context menu close with 100ms delay to allow time to prevent clicks
    - handleRightClickEnd decides per menu if to close and set those cleanup things or not, like for filter it doesn't if clicked anywhere in context menu by looping over html elements and their parents
- pipelineId renamed to piprlineExecutionId for more sense
multiple pipelines:
    - eda from context now gets exact ast node of placeholder with range of the executed context and from there the pipeline container => pipeline name
    - pipeline name is passed to execute pipeline which is now needed
    - also sent to eda where 1. it is used for tableIdentifier as pipelineName + '.' + tableName (tableName new param used and passed outside of tableIdentifier) and 2. it is passed to runner for pipeline execution
    - in Runner the pipeline in question is found and in front of it's closing } the new code is then added
- getStateByPlaceholder now getTableByPlaceholder and transform to state obj in eda class, whee table name and table identifier are known, this method now only relevant stuff
- createOrShow async as well as calling register command methods
- runnerApi now instance var of panel
- got rid of an excess profiling banner div:
    - Now apparently now methods for table resize/TableSpace needed anymore, all handled by html itself
    - Meaning I also cannot change much about that mechanism, other than the min width, set by setting width on startup of the elements
    - Also means a lot less code and complexity!
    - savedColumnWidths now is a svelte store, that the table subscribes to, meaning it updates correctly, only needed on resize and reorder (when letting the col go); also now no manual setting of stlyle for this anymore
    - the automatic handling by html meant that reordering did not properly take out of table, so now a "reorderPrototype" of a column header that is used for the under cursor display and updates with relevant data, while column is made "display: none" in table
    - Min width maybe as initial width if it is being streched, then increasing size of a col will not result in others shrinking, but how to decide if in full view or not?
    - Also full view makes scrolling for fixed stuff lag?? Mabye visible scroll bar or extra tiny div
- Fixed that full view makes fixed stuff lag by making table width 100.1% instead of 100%, so always tiny bit out of view that causes scroll to exist
- increased scroll buffer a bit to make more fluent
- now you cannot see the table text through the borders of the headers/profiling anymore if table scrolled
    - have an absolute div at 100% with at top that is bright bg color
normal height = 2 * rowHeight
    - if profiling expanded then delayed (since height animation of profilingInfo) setting of height to 2 * rowHeight + profilingInfo height, not complete height as not including for example borders but enough to cover all bg space that let's text through
    - top prop of this also = scrollTop
- changed profiling to always include value, not name, as we display values
- thus image string also as "value" as well as string when just using "text" type (prev. "name" type)
- profiling placeholder name gen now not random but with codegen prefix + incr counter number
@SmiteDeluxe SmiteDeluxe added enhancement 💡 New feature or request vscode 🔨 Issues regarding tools like the VS Code extension labels May 9, 2024
@SmiteDeluxe SmiteDeluxe changed the title 955 eda plot view feat: eda plot view May 9, 2024
Copy link

github-actions bot commented May 9, 2024

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ CSS stylelint 1 0 0 1.71s
✅ JSON jsonlint 1 0 0.19s
✅ JSON npm-package-json-lint yes no 0.75s
✅ JSON prettier 21 0 0 3.36s
✅ JSON v8r 1 0 2.55s
✅ REPOSITORY git_diff yes no 0.25s
✅ TSX eslint 19 0 0 9.73s
✅ TYPESCRIPT eslint 9 0 0 8.44s
✅ TYPESCRIPT prettier 9 0 0 1.55s

See detailed report in MegaLinter reports
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

MegaLinter is graciously provided by OX Security

Copy link

codecov bot commented May 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (1a24a18) to head (7364077).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main     #1161   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          110       110           
  Lines        19238     19238           
  Branches      4114      4114           
=========================================
  Hits         19238     19238           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@SmiteDeluxe
Copy link
Contributor Author

@lars-reimann a note on this: Creating a boxplot on a non-numeric column throws an error. For other plots this works fine. Should I manually disallow boxplots for categorical columns (with my limited interpretation of categorical vs numerical atm) or is there smth to be done elsewhere?

@lars-reimann
Copy link
Member

lars-reimann commented May 9, 2024

@SmiteDeluxe There are several options:

I don't expect major changes to this, other than dropping the prefix "Experimental" from the name.

We could also add the options ignore_errors and ignore_warnings to the program (soon run) message (=> Safe-DS/Runner#113).

@SmiteDeluxe
Copy link
Contributor Author

@lars-reimann Will go wit the is_numeric then for now and disallow boxplots for other columns. Will do a new PR for that after this is merged!

@lars-reimann
Copy link
Member

@lars-reimann Will go wit the is_numeric then for now and disallow boxplots for other columns. Will do a new PR for that after this is merged!

Alright. I probably won't be able to review this PR before the weekend.

@lars-reimann
Copy link
Member

Although, maybe I'll find time to review it tomorrow morning and do another release for the lab. Looks awesome at first glance!

@SmiteDeluxe
Copy link
Contributor Author

@lars-reimann for this: ExperimentalDataType.is_numeric

Do I need to define my own stubs?

@lars-reimann
Copy link
Member

lars-reimann commented May 10, 2024

Nope, it's in the latest release.

Copy link
Member

@lars-reimann lars-reimann left a comment

Choose a reason for hiding this comment

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

No suggestions, very nicely done!

@lars-reimann lars-reimann merged commit a216743 into main May 11, 2024
8 checks passed
@lars-reimann lars-reimann deleted the 955-eda-plot-view branch May 11, 2024 20:28
lars-reimann pushed a commit that referenced this pull request May 16, 2024
## [0.16.0](v0.15.0...v0.16.0) (2024-05-16)

### Features

* eda plot view ([#1161](#1161)) ([a216743](a216743)), closes [#955](#955) [#986](#986)
* integrate version 0.25.0 of the `safe-ds` Python library ([#1174](#1174)) ([f357c38](f357c38))
* prefix keywords with `^` to treat them as identifiers ([#1172](#1172)) ([90bd47c](90bd47c))

### Bug Fixes

* potential stack overflow when computing types of lambda parameters ([#1173](#1173)) ([d89511e](d89511e))
@lars-reimann
Copy link
Member

🎉 This PR is included in version 0.16.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@lars-reimann lars-reimann added the released Included in a release label May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 💡 New feature or request released Included in a release vscode 🔨 Issues regarding tools like the VS Code extension
Projects
Status: ✔️ Done
Development

Successfully merging this pull request may close these issues.

EDA: Remove old vs-code states EDA: Plot view
4 participants