-
Notifications
You must be signed in to change notification settings - Fork 0
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(vis): violin vis selection events #267
Conversation
Thanks for this PR @dvmartinweigl! I have tested it in BioInsight, and for me it works as expected 馃憤 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested this PR in Ordino and it does not work from the first look.
Steps to reproduce
- Open Cell line workbench
- Add Micro Satellite Instability (MSI) score
- Open Configurable charts view
- Switch to violin plot
- Select Gender as categorical column
Observations
Ordino DEV
- The violin plot has colors (even though they don't match the gender categorical colors)
- Tooltips on hover are working
- Selecting the male violin plot selects rows in the data table BUT it selects also female rows (which is clearly wrong) --> no highlight of the male violin plot
- Selection in the data table is not reflected in the violin plot (i.e., no highlight)
Local version (with this PR)
Shows the following error in the dev console on hover:
Uncaught TypeError: Cannot read properties of undefined (reading '0')
at _ (plotly.min.js:37:1)
at O (plotly.min.js:37:1)
at r.getClosest (plotly.min.js:37:1)
at Object.l [as hoverOnBoxes] (plotly.min.js:37:1)
at e1.exports [as hoverPoints] (plotly.min.js:37:1)
at ut (plotly.min.js:37:1)
at plotly.min.js:37:1
at plotly.min.js:37:1
at l (plotly.min.js:37:1)
at r.throttle (plotly.min.js:37:1)
- No colored violin plots
- No tooltips on hover -> instead error above
- Selecting a violin plot has no effect but repeating the error message
- Selecting some rows in the data table always highlights the first violin plot (independent if only male or female or mixed are selected)
Expected result
The error should be fixed and the selection must work again. In best case the categorical colors are also matching the one in the LineUp.
Local dev setup
@dvchristianbors Steps to run the setup due to the missing merge and release of datavisyn/visyn_scripts#67.
package.json resolutions
{
"resolutions": {
"visyn_core": "git+ssh://[email protected]/datavisyn/visyn_core.git#mweigl/add-proper-violin-plot-selection",
"visyn_scripts": "git+ssh://[email protected]/datavisyn/visyn_scripts.git#mweigl/remove-security-routes-proxy"
}
}
requirements.txt --> add the following two dependencies:
visyn_core@git+https://github.com/datavisyn/visyn_core.git@develop#egg=visyn_core
# pin werkzeug version because the new major version 3.0.0 breaks our applications on Oct 2 2023; the version can be removed later again when the other packages support v3.0.0
werkzeug==2.3.7
mock_data_api/requirements.txt
uvicorn[standard]==0.29.0
fastapi==0.110.1
Run yarn install
and make develop
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your changes.
I reviewed @dvmartinweigl 's latest changes:
No colored violin plots
Will be covered in a follow-up PR that also addresses synchronizing colors between a possible ranking and the vis.
No tooltips on hover -> instead error above
Error no longer occurs
Selecting a violin plot has no effect but repeating the error message
Selection now works (also multi-select)
Selecting some rows in the data table always highlights the first violin plot (independent if only male or female or mixed are selected)
Selections in the ranking will highlight the correct violins, accordingly.
Another comment from my side:
As soon as there is a large number of violin plots, the individual plots no longer can be hovered or selected. However, this seems to be a limitation within plotly itself, and is only really solvable by the user specifically filtering out categories. I think this is fine, since for a general vis, we can not realistically cover all edge cases.
@dvchristianbors tested the most recent changes on behalf of me
## What's changed * feat: add possibility to insert score column after given column in ranking (#262) * refactor: rename labels of filter buttons (#266) * fix(vis): duplicate option values in scatterplot sidebar select (#261) * chore: remove usage of flask (#257) * fix(vis): violin vis selection events (#267)
Partially fixed: #227
Developer Checklist (Definition of Done)
Issue
UI/UX/Vis
Code
PR
release: minor
) to this PR following semverCloses #...
)Summary of changes
Screenshots
Additional notes for the reviewer(s)
Thanks for creating this pull request 馃