Texera / texera

Collaborative Machine-Learning-Centric Data Analytics Using Workflows

https://texera.github.io

Apache License 2.0

160 stars 68 forks source link

Add Log Plot Support to Scatter Plot Visualization Operator #2712

Closed sixsage closed 2 months ago

sixsage commented 2 months ago

Purpose

Modify the scatter plot operator to support log-scaling in the X and/or Y axis

Description of Changes

As suggested by kunwp1, log plot was originally a visualization operator on its own but decided to merge it to scatter plot operator instead. The log-scaling arguments were found here

ScatterplotOpDesc.scala: implement log-scaling into the original scatter plot operator

kunwp1 commented 2 months ago

Hey @sixsage, I wanted to let you know that your PR looks excellent! My only suggestion is that the plot appears similar to the existing scatter plot. Both use the Plotly API but with different parameters. Instead of creating the log plot as a new visualization operator, it would be nice to integrate your work into the scatter plot. I'm curious to hear your thoughts on this. Thank you.

sixsage commented 2 months ago

Thank you for bringing that up. I agree that the log plot can be integrated into the scatter plot operator and I will make the change. Should I close this PR and start a new PR when I am done or should I keep this open?

sixsage commented 2 months ago

Hello @kunwp1 - I have made the changes you have kindly suggested. If you have the time it would be great if you could review my PR once more. Thank you!