-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
First Push for Trisurf Plots #453
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
Conversation
Some more color variety for y'all.
HECK YEAH! These are looking great. Can't wait to replace the mammoth 🐘 we have here:
https://plot.ly/python/trisurf/
With some slick 1 liners :)
plotly/tools.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🐄 This should go to the top of the file. The above constants should have been at the top as well.
@Kully Looking amazing! Mostly small comments about style and organization. Ping me when tests are up for another review!
...ith Python 2, 3.3 and 3.4
In python 3 `map` is a generator, `list()` it!
...nto Trisurf_Plots
@theengineear @jackparmer @cldougl @chriddyp
Final something with no errors. Thanks a lot to Andrew for helping me out with this last night.
Checklist:
-Pep-8 compliant
-tests are thorough and work
Great! I'll take a peek today!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😿 Can you reformat this? You could even save on indentation by defining it at the top of the file or something. For example, this would be much better:
{
'data': [
{
'facecolor': ['rgb(143.0, 123.0, 97.000000000000014)',
'rgb(255.0, 127.0, 14.000000000000007)',
'rgb(143.0, 123.0, 97.000000000000014)',
'rgb(31.0, 119.0, 180.0)',
'rgb(143.0, 123.0, 97.000000000000014)',
'rgb(31.0, 119.0, 180.0)',
'rgb(143.0, 123.0, 97.000000000000014)',
'rgb(255.0, 127.0, 14.000000000000007)'],
'i': [3, 1, 1, 5, 7, 3, 5, 7],
'j': [1, 3, 5, 1, 3, 7, 7, 5],
'k': [4, 0, 4, 2, 4, 6, 4, 8],
'name': '',
'type': 'mesh3d',
'x': np.array([-1., 0., 1., -1., 0., 1., -1., 0., 1.]),
'y': np.array([-1., -1., -1., 0., 0., 0., 1., 1., 1.]),
'z': np.array([ 1., -0., -1., -0., 0., 0., -1., 0., 1.])
},
{
'line': {'color': 'rgb(50, 50, 50)', 'width': 1.5},
'mode': 'lines',
'type': 'scatter3d',
'x': [-1.0, 0.0, 0.0, -1.0, None, 0.0, -1.0, -1.0, 0.0, None, 0.0,
1.0, 0.0, 0.0, None, 1.0, 0.0, 1.0, 1.0, None, 0.0, -1.0, 0.0,
0.0, None, -1.0, 0.0, -1.0, -1.0, None, 1.0, 0.0, 0.0, 1.0,
None, 0.0, 1.0, 1.0, 0.0, None],
'y': [0.0, -1.0, 0.0, 0.0, None, -1.0, 0.0, -1.0, -1.0, None, -1.0,
0.0, 0.0, -1.0, None, 0.0, -1.0, -1.0, 0.0, None, 1.0, 0.0,
0.0, 1.0, None, 0.0, 1.0, 1.0, 0.0, None, 0.0, 1.0, 0.0, 0.0,
None, 1.0, 0.0, 1.0, 1.0, None],
'z': [-0.0, -0.0, 0.0, -0.0, None, -0.0, -0.0, 1.0, -0.0, None,
-0.0, 0.0, 0.0, -0.0, None, 0.0, -0.0, -1.0, 0.0, None, 0.0,
-0.0, 0.0, 0.0, None, -0.0, 0.0, -1.0, -0.0, None, 0.0, 0.0,
0.0, 0.0, None, 0.0, 0.0, 1.0, 0.0, None]
}
],
'layout': {
'height': 800,
'scene': {'aspectratio': {'x': 1, 'y': 1, 'z': 1},
'xaxis': {'backgroundcolor': 'rgb(230, 230, 230)',
'gridcolor': 'rgb(255, 255, 255)',
'showbackground': True,
'zerolinecolor': 'rgb(255, 255, 255)'},
'yaxis': {'backgroundcolor': 'rgb(230, 230, 230)',
'gridcolor': 'rgb(255, 255, 255)',
'showbackground': True,
'zerolinecolor': 'rgb(255, 255, 255)'},
'zaxis': {'backgroundcolor': 'rgb(230, 230, 230)',
'gridcolor': 'rgb(255, 255, 255)',
'showbackground': True,
'zerolinecolor': 'rgb(255, 255, 255)'}},
'title': 'Trisurf Plot',
'width': 800
}
}
Still looking good to me! Ping me when you're all finished and I'll give it a final review.
It would be great to do a sanity check making sure this figure_factory
still works smoothly with PLY files.
http://moderndata.plot.ly/visualizing-56-ply-files-in-python/
Is custom surface coloring handled?
https://plot.ly/python/3d-surface-coloring/
On Tue, May 10, 2016 at 9:59 AM, Andrew notifications@github.com wrote:
Still looking good to me! Ping me when you're all finished and I'll give
it a final review.—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#453 (comment)
It would be great to do a sanity check making sure this
figure_factory
still works smoothly with PLY files.http://moderndata.plot.ly/visualizing-56-ply-files-in-python/
Is custom surface coloring handled?
No, custom coloring isn't handled yet, but I can add.
I can check the PLT files as well.
Awesome - Beethoven in the house! Last thing we need is support for custom
coloring. Lots of customers need coloring to change with a variable that is
not the Z axis.
On Tue, May 10, 2016 at 1:52 PM, Adam Kulidjian notifications@github.com
wrote:
@jackparmer https://github.com/jackparmer It's more than working... =D
[image: trisurf plot - chopper]
https://cloud.githubusercontent.com/assets/10369095/15162147/80a9fe2c-16cf-11e6-9a92-1ce43fbba794.png[image: trisurf plot - beethoven]
https://cloud.githubusercontent.com/assets/10369095/15162153/84b5118c-16cf-11e6-80dc-77c6cd142a06.png—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#453 (comment)
Awesome - Beethoven in the house! Last thing we need is support for custom
coloring. Lots of customers need coloring to change with a variable that is
not the Z axis.
Makes sense and I'm on it.
@jackparmer @theengineear @cldougl
I have added the option to write a function dist_func(x, y, z) and input that into the Trisurf function.
A couple notes:
-For my examples in the create_trisurf doc string, they only work in my notebook if I do the usual cd adam/plotly/plotly.py (i.e. move to the appropriate directory). I don't think I had to add something like that for a doc like this, as I imagine the user will have access to the plotly module from anywhere.
-I had previously added a from scipy.spatial import Delaunay line to test_figure_factory.py in test_optional (which was there in the previous PR update) so that my trisurf tests would work. Just a note.
For my examples in the create_trisurf doc string, they only work in my notebook if I do the usual cd adam/plotly/plotly.py (i.e. move to the appropriate directory). I don't think I had to add something like that for a doc like this, as I imagine the user will have access to the plotly module from anywhere.
AH! That's why you don't need the special activate file for your venv. Makes sense! Also, nope, don't include that in the docstring. The user should be A-OK.
I had previously added a from scipy.spatial import Delaunay line to test_figure_factory.py in test_optional (which was there in the previous PR update) so that my trisurf tests would work. Just a note.
Okie dokie.
No new notes from me, I think there are a few code comments in https://github.com/plotly/plotly.py/pull/453/files that were not addressed but nothing major.
Not gonna block this one --> 💃
@jackparmer @theengineear @cldougl
This is the PR for create_trisurf() in FigureFactory.
The functionality so far is this: it takes arrays x, y and z (the vertices of the points) which are to be created outside the function. This requires a _import numpy _and from scipy.spatial import Delaunay in the notebook.
It looks pretty cool right now. Like the scatterplotmatrix, you can enter a Plotly colorscale name in colorscale= to get your heatmap as a function of the z-coordinate. Or you can opt to enter a list of 2 rgb tuples (or normalized rgb tuples, where each coordinate is between 0 and 1).
I also included a doc string which outputs some cool looking plots.
Things I need to do:
-add tests (which I can do now!)
-potential **kwargs
Bring forth your comments!