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(vis-violin): violin vis subcategories and proper facets #331

Merged
merged 100 commits into from
Jun 14, 2024

Conversation

dvmartinweigl
Copy link
Contributor

@dvmartinweigl dvmartinweigl commented Jun 6, 2024

Closes https://github.com/datavisyn/bioinsight/issues/437

Developer Checklist (Definition of Done)

Issue

  • All acceptance criteria from the issue are met
  • Tested in latest Chrome/Firefox

UI/UX/Vis

  • Requires UI/UX/Vis review
    • Reviewer(s) are notified (tag assignees)
    • Review has occurred (link to notes)
    • Feedback is included in this PR
    • Reviewer(s) approve of concept and design

Code

  • Branch is up-to-date with the branch to be merged with, i.e., develop
  • Code is cleaned up and formatted
  • Unit tests are written (frontend/backend if applicable)
  • Integration tests are written (if applicable)

PR

  • Descriptive title for this pull request is provided (will be used for release notes later)
  • Reviewer and assignees are defined
  • Add type label (e.g., bug, feature) to this pull request
  • Add release label (e.g., release: minor) to this PR following semver
  • The PR is connected to the corresponding issue (via Closes #...)
  • Summary of changes is written

Summary of changes

  • added various features and improvements to violin vis
    • possibility to select subgroup column
    • create facets for data by selected column
    • added sort icons --> sort by mean (yAxis) and alphabetically (xAxis)
    • added possibility to sync the yAxis across subplots
    • added strip overlay with possibilty to select single points or whole violins
    • consistent style for selection state using opacity + group colors for selected violins
  • added breast cancer dataset to cover more test and use cases for vis

Screenshots

image

image

image

image

Additional notes for the reviewer(s)

Thanks for creating this pull request 🤗

Copy link
Contributor

@dvchristianbors dvchristianbors left a comment

Choose a reason for hiding this comment

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

Thanks for your PR. I just made a tiny change, pulling the traces color into a constant. Otherwise, it looks good to me. I tested several permutations of the violin plot. And with the addition of the new breast cancer sample dataset, everything worked nicely (also some columns with missing data).

@dvdanielamoitzi
Copy link
Contributor

dvdanielamoitzi commented Jun 12, 2024

@dvmartinweigl thanks for this PR, I have a few minor comments:

  • please add a tooltip when the facets is disabled explaining why:
    image

  • Can we reduce the horizontal spacing a little? There's so much whitespace:
    image

@dvchristianbors
Copy link
Contributor

Violin plot stories are currently broken, this will be addressed in a follow-up ticket: #345

@dvdanielamoitzi
Copy link
Contributor

dvdanielamoitzi commented Jun 12, 2024

  • when we have split violins with 2 categories, and the left violin is null, the right violin takes the place of the left violin:
    image

  • we should consider the number of categories for the spacing when we do faceting. E.g. when we have more than 2 in a row / column, the space should be reduced. -> same for the scatter vis

  • sorting by the category on the x-axis shows all categories, even if they are null. When the axis is unsorted, it correclty shows the right amount of categories. (As discussed with @dvmartinweigl this could be tricky and we'd create a follow-up ticket for it.)

@thinkh thinkh changed the title Feature: violin vis subcategories and proper facets feat(vis-violin): violin vis subcategories and proper facets Jun 13, 2024
@dvchristianbors dvchristianbors self-requested a review June 14, 2024 08:37
Copy link
Contributor

@dvchristianbors dvchristianbors left a comment

Choose a reason for hiding this comment

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

Thanks for your final changes. The last issue that caused the violin plot not to be shown in Ordino now properly works. From my point of view, this PR can now be merged.

@dvchristianbors dvchristianbors merged commit d429dc2 into develop Jun 14, 2024
10 checks passed
@dvmartinweigl dvmartinweigl mentioned this pull request Jun 17, 2024
17 tasks
@dvvanessastoiber dvvanessastoiber mentioned this pull request Jun 19, 2024
puehringer added a commit that referenced this pull request Jun 19, 2024
## What's changed

* feat(vis): align visual appearance, rotate bar plot axis labels, add
vis hooks, remove raincloud plot
(#293)
* feat(vis-violin): violin vis subcategories and proper facets
(#331)
* feat: add console integration for Sentry
(#347)
* feat: add boxplot vis type
(#302)
* deps: upgrade deps to latest versions (20240611)
(#344)
* feat(vis): add column description to selects and pills
(#358)
* feat: lazy load plotly where possible
(#346)
* feat: add BlurredOverlay component
(#359)
* Fix: export vis color and fix y-axis label
(#360)
@thinkh thinkh deleted the mweigl/violin-vis-grouping branch June 24, 2024 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release: patch PR merge results in a new patch version type: feature New feature or request vis-type: violin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants