Skip to content

Conversation

@qzt168
Copy link
Contributor

@qzt168 qzt168 commented Apr 8, 2025

Purpose:

  • This PR adds Violin Plot Operator support to the existing Box Plot Operator.
  • It allows users to switch between Box Plot and Violin Plot using a checkbox.

Changes:

In core/workflow-operator/src/main/scala/edu/uci/ics/amber/operator/visualization/boxViolinPlot/BoxViolinPlotOpDesc.scala:

  • Added a new parameter violinplot with a checkbox.
  • Made the operator draw a violin plot instead of a box plot when violinplot is selected.
  • Updated createPlotlyFigure() to support both plot types.
  • Updated the user-friendly operator name (Box Plot -> Box/Violin Plot).

In core/workflow-operator/src/main/scala/edu/uci/ics/amber/operator/LogicalOp.scala:

  • Updated registration name (BoxPlot -> BoxViolinPlot).

In core/gui/src/assets/operator_images/BoxViolinPlot.png:

  • Updated the operator image.

Dataset for Test:

tips.csv

ViolinPlot
image

@qzt168 qzt168 added the feature label Apr 8, 2025
@qzt168 qzt168 requested a review from GspikeHalo April 8, 2025 08:09
@qzt168 qzt168 self-assigned this Apr 8, 2025
@qzt168
Copy link
Contributor Author

qzt168 commented Apr 9, 2025

I have updated the registration name and image of the BoxPlot operator to BoxViolinPlot.
The updated files are:

  • core/gui/src/assets/operator_images/BoxViolinPlot.png
  • core/workflow-operator/src/main/scala/edu/uci/ics/amber/operator/visualization/boxViolinPlot/BoxViolinPlotOpDesc.scala
  • core/workflow-operator/src/main/scala/edu/uci/ics/amber/operator/LogicalOp.scala
image

@GspikeHalo
Copy link
Contributor

I have updated the registration name and image of the BoxPlot operator to BoxViolinPlot. The updated files are:

  • core/gui/src/assets/operator_images/BoxViolinPlot.png
  • core/workflow-operator/src/main/scala/edu/uci/ics/amber/operator/visualization/boxViolinPlot/BoxViolinPlotOpDesc.scala
  • core/workflow-operator/src/main/scala/edu/uci/ics/amber/operator/LogicalOp.scala
image

Please directly modify the original description without adding any extra content.

@GspikeHalo
Copy link
Contributor

Please add the dataset used for testing in the description of this PR.

Copy link
Contributor

@GspikeHalo GspikeHalo left a comment

Choose a reason for hiding this comment

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

LGTM!

@qzt168 qzt168 merged commit 1658865 into master Apr 10, 2025
8 checks passed
@qzt168 qzt168 deleted the qizhi-add-violinplot-operator branch April 10, 2025 01:27
@GspikeHalo GspikeHalo changed the title Qizhi add violinplot operator Add violinplot operator Apr 10, 2025
Copy link
Contributor

@Xiao-zhen-Liu Xiao-zhen-Liu left a comment

Choose a reason for hiding this comment

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

I left a few more additional code comments. Please create anther PR to address them.

In addition, here are a few other comments on the PR title and description:

  • The PR "adds Violin Plot Operator support to the existing Box Plot Operator." Since you are not creating a new operator, please change the PR title to something like "Add ViolinPlot Option to BoxPlot Operator".
  • PR descriptions should be treated like formal documentations and should not have grammar issues. In your "Changes" section of the PR description, make sure your bullet points have consistent tense and format. For example, "Updated createPlotlyFigure() to support both plot types." and "Update the user-friendly operator name (Box Plot -> Box/Violin Plot)" differ both in tense and type (one has a comma while the other does not).

@qzt168 qzt168 changed the title Add violinplot operator Add ViolinPlot Option to BoxPlot Operator Apr 10, 2025
qzt168 added a commit that referenced this pull request Apr 12, 2025
…lot operator (#3365)

This PR fixes a previous PR (#3356), and includes the following updates:

- Renamed variables for better clarity and consistency:
  - orientation → horizontalOrientation
  - violinplot → violinPlot
  - quartiletype → quartileType
- Updated the Violin Plot checkbox description to clarify its behavior
- Revised the operator description to explain that the violin plot
overlays the box plot for better visualization
- Modified the createPlotlyFigure() method:
  - Now supports `quartileType` when `box=True` in a violin plot
- Removed `jitter=0` to allow data points to spread naturally instead of
forming a straight line — this shows the distribution shape better
<img width="1439" alt="image"
src="https://github.com/user-attachments/assets/b8e96412-aadb-4b96-97fb-16eb6a67179b"
/>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants