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 :mesh3d series type for Plotly #2909

Merged
merged 19 commits into from
Aug 14, 2020

Conversation

dwd31415
Copy link
Contributor

This pull-request adds the possibility to plot arbitrary 3d meshes using the mesh3d plot type in Plotly. It can be seen as partial fix/implementation for #392. For backends other than Plotly there is a fallback recipe provided that however does not support specifying the triangles of the mesh.

Example:

using Plots; plotly()

x=[0, 1, 2, 0]
y=[0, 0, 1, 2]
z=[2, 4, 2, 3]

i=[0, 0, 0, 1]
j=[1, 2, 3, 2]
k=[2, 3, 1, 3]

plot(x,y,z,seriestype=:mesh3d;i,j,k)

Output:
image

Copy link
Member

@BeastyBlacksmith BeastyBlacksmith left a comment

Choose a reason for hiding this comment

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

Hello Adrian,
thanks for this contribution. That is a really good feature and a worthwhile addition to Plots.jl.

There are some things we need to adjust. Firstly, the API is not really in line with what Plots usually does and i, j, k are hard to discover keywords. So I would suggest we add a new series keyword connections (better names welcome) and pass a tuple of vectors like

mesh3d(x, y, z, connections = (i, j, k))

In the future we then could also support vectors of tuples and matrices.

Secondly it would be good to have an example in the examples file, so this gets added to the tests and shows up in the documentation and there should also be an explanation of what the values of i, j and k actually refer to.

src/pipeline.jl Outdated Show resolved Hide resolved
src/recipes.jl Outdated Show resolved Hide resolved
@dwd31415
Copy link
Contributor Author

Hi Simon,
thanks for the feedback.

Regarding the first point that sounds like a much cleaner interface. Am I right that adding connection as a series keyword would be achieved by adding it to _series_defaults in args.jl? It seems to work but I just wanted to check it is not some kind of hacky way.

@BeastyBlacksmith
Copy link
Member

Am I right that adding connection as a series keyword would be achieved by adding it to _series_defaults in args.jl? It seems to work but I just wanted to check it is not some kind of hacky way.

You are right. That is the way to go.

src/examples.jl Outdated Show resolved Hide resolved
src/recipes.jl Outdated Show resolved Hide resolved
Copy link
Member

@BeastyBlacksmith BeastyBlacksmith left a comment

Choose a reason for hiding this comment

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

This should be the last round of changes and its mainly cosmetics

src/args.jl Outdated Show resolved Hide resolved
src/backends/plotly.jl Outdated Show resolved Hide resolved
src/examples.jl Outdated Show resolved Hide resolved
src/pipeline.jl Outdated Show resolved Hide resolved
src/pipeline.jl Outdated Show resolved Hide resolved
src/shorthands.jl Outdated Show resolved Hide resolved
dwd31415 and others added 4 commits August 14, 2020 13:04
@dwd31415
Copy link
Contributor Author

Okay great, then everything should be fine now.

@BeastyBlacksmith
Copy link
Member

Thanks!

@BeastyBlacksmith BeastyBlacksmith merged commit e2c3878 into JuliaPlots:master Aug 14, 2020
@dlfivefifty
Copy link
Contributor

Does this support setting the colours of the triangle? Trying to plot a function a sphere: have the sphere working but now need the colours

@dwd31415
Copy link
Contributor Author

dwd31415 commented May 8, 2021

No I don't think this is supported. As a hacky workaround you could plot the triangles individually since you can specify the color of the whole mesh like this mesh3d(x,y,z;color=:purple,connections=(i,j,k))

@dlfivefifty
Copy link
Contributor

That's too bad... I'm guessing the Plotly backend supports it?

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.

3 participants