Skip to content

First pass at updating API for allowing animations#339

Open
nicrummel wants to merge 2 commits into
JuliaPlots:masterfrom
nicrummel:animation
Open

First pass at updating API for allowing animations#339
nicrummel wants to merge 2 commits into
JuliaPlots:masterfrom
nicrummel:animation

Conversation

@nicrummel

Copy link
Copy Markdown

Update display function to allow for frames for animation, and add animation example

@nicrummel

Copy link
Copy Markdown
Author

I see that the Labeler is failing, is there a way I can address this?

@sglyon sglyon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for making an attempt at this! It will be a welcome and helpful contribution.

I have one question about the change to calling the javascript Plotly.newPlot function, but other than that this looks great.

Comment thread src/display.jl Outdated
# Draw plot in container
Plotly.newPlot(
gd, $(lowered[:data]), $(lowered[:layout]), $(options)
gd, $(lowered), $(options)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does this work as expected?

From the plotly.js docs (ref) it looks like we can use one of two forms of the newPlot function:

Plotly.newPlot(graphDiv, data, layout, config)

Plotly.newPlot(graphDiv, obj)

Here I don't believe we are using either

@nicrummel

Copy link
Copy Markdown
Author

I looked at the documentation, and I believe you caught a mistake. I have fixed the call in the most recent commit.

@sglyon

sglyon commented Aug 18, 2020

Copy link
Copy Markdown
Member

Now that the PlotlyBase side has been merged, hopefully I'll be able to review this one soon.

Thanks for the contributions and for the patience!

@nicrummel

nicrummel commented Sep 10, 2020

Copy link
Copy Markdown
Author

Dr. Lyon,
I see you have been quite busy with the development of other aspects of PlotlyJS.jl. Is ther anything I can do to help get this pull request along?

  • Possibly remove the example and docs that I made? Should I move those to another location?

  • I could also rebaise my branch on the current master. Would that help?

Regards,
Nic

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