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

Add Support For 3D Scatter Plots and Line Plots (Closes #1463) #1572

Merged
merged 14 commits into from
Apr 3, 2020

Conversation

umesh-timalsina
Copy link
Contributor

  1. Add support for 3DScatter and Line Plots.
  2. Refactor PlotlyDescExtractor to incoroprate3D Subplots
  3. Extract 3D plots data from backend_deepforge.py
  4. Minor changes to ExectionIndexControl
  5. Change plotly.min.js to new version (v1.52.3)
  6. Other minor changes

1. Add support for 3DScatter and LinePlots.
2. Refactor PlotlyDescExtractor to incoroprate3D Subplots
3. Extract 3D plots data from backend_deepforge.py
4. Minor changes to ExectionIndexControl
5. Change plotly.min.js to new version (v1.52.3)
6. Other minor changes
@brollb
Copy link
Contributor

brollb commented Apr 2, 2020

Would you mind adding an example pipeline to devProject? It might be nice to start adding a pipeline for testing some of the job metadata. (It would also make it easier to review :) )

Copy link
Contributor

@brollb brollb left a comment

Choose a reason for hiding this comment

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

Nice work. A couple minor changes. There are a few parts where it might be a little more complicated and I suspect it could be simplified but it's not a huge deal.

I still need to test it out locally but as long as there are no surprises, we should be good to go after the two minor comments are addressed!


desc = {
id: id,
execId: execId,
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be nice to reduce the code duplication between 2D and 3D plots. The only gotcha is that 3D plots don't support images. Maybe they could be refactored to add a method for the shared abstract base class in the metamodel and both could call it...

src/common/viz/FigureExtractor.js Outdated Show resolved Hide resolved
@brollb
Copy link
Contributor

brollb commented Apr 2, 2020

It looks like the graph container is not filling the entire space... This may not be related to 3D support (and may be better to address in its own issue/PR) but worth mentioning:
DeepinScreenshot_select-area_20200402151608

@brollb brollb added this to the 2.2.0 milestone Apr 3, 2020
@brollb brollb merged commit 5c3aeb4 into master Apr 3, 2020
@brollb brollb deleted the 1463-3d-plots branch April 3, 2020 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants