Skip to content

added appropriate colorbar max and min values, edges_color to param for line coloring, and making code more clear #551

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

Merged
merged 4 commits into from
Aug 22, 2016
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 36 additions & 19 deletions plotly/tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -3269,9 +3269,9 @@ def _map_face2color(face, colormap, vmin, vmax):
return face_color

@staticmethod
def _trisurf(x, y, z, simplices, show_colorbar, colormap=None,
color_func=None, plot_edges=False, x_edge=None, y_edge=None,
z_edge=None, facecolor=None):
def _trisurf(x, y, z, simplices, show_colorbar, edges_color,
colormap=None, color_func=None, plot_edges=False,
x_edge=None, y_edge=None, z_edge=None, facecolor=None):
"""
Refer to FigureFactory.create_trisurf() for docstring
"""
Expand Down Expand Up @@ -3305,7 +3305,7 @@ def _trisurf(x, y, z, simplices, show_colorbar, colormap=None,
color_func[index] = FigureFactory._label_rgb(foo)

mean_dists = np.asarray(color_func)
else:
elif hasattr(color_func, '__call__'):
Copy link
Contributor

Choose a reason for hiding this comment

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

🐄 Why not just elif color_func is not None?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why not just elif color_func is not None?

Your way makes sense upon closer inspection. Actually, I think else would work as well, since if color_func was a function, it would therefore not be None nor a list/array

# apply user inputted function to calculate
# custom coloring for triangle vertices
mean_dists = []
Expand Down Expand Up @@ -3333,30 +3333,39 @@ def _trisurf(x, y, z, simplices, show_colorbar, colormap=None,
max_mean_dists)
facecolor.append(color)

# Make sure we have arrays to speed up plotting
# Make sure facecolor is a list so output is consistent across Pythons
facecolor = list(facecolor)
ii, jj, kk = simplices.T

# make a colorscale from the colors
colorscale = FigureFactory._make_colorscale(colormap)
colorscale = FigureFactory._convert_colorscale_to_rgb(colorscale)

triangles = graph_objs.Mesh3d(x=x, y=y, z=z, facecolor=facecolor,
i=ii, j=jj, k=kk, name='')

colorbar = graph_objs.Mesh3d(x=[0, 1], y=[0, 1], z=[0, 1],
colorscale=colorscale,
intensity=[0, 1],
showscale=True)
if not isinstance(mean_dists[0], str) and show_colorbar is True:
# make a colorscale from the colors
colorscale = FigureFactory._make_colorscale(colormap)
colorscale = FigureFactory._convert_colorscale_to_rgb(colorscale)

colorbar = graph_objs.Scatter3d(
x=x[:2],
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you comment on why we ditch the first couple values here? I can't remember why we'd want to do this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Edit only keep not ditch, sorry.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just looking for some documentation on it 😸

y=y[:2],
z=z[:2],
mode='markers',
marker=dict(
size=0.1,
color=[min_mean_dists, max_mean_dists],
colorscale=colorscale,
showscale=True),
hoverinfo='None',
showlegend=False
)

# the triangle sides are not plotted
if plot_edges is not True:
if show_colorbar is True:
if plot_edges is False:
Copy link
Contributor

Choose a reason for hiding this comment

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

😛 👍

if not isinstance(mean_dists[0], str) and show_colorbar is True:
Copy link
Contributor

@theengineear theengineear Aug 19, 2016

Choose a reason for hiding this comment

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

🐄 you use this not isinstance(mean_dists[0], str) twice thrice, can you assign it to a meaningful variable name to make the code easier to understand?

return graph_objs.Data([triangles, colorbar])
else:
return graph_objs.Data([triangles])


# define the lists x_edge, y_edge and z_edge, of x, y, resp z
# coordinates of edge end points for each triangle
# None separates data corresponding to two consecutive triangles
Expand Down Expand Up @@ -3392,10 +3401,14 @@ def _trisurf(x, y, z, simplices, show_colorbar, colormap=None,
# define the lines for plotting
lines = graph_objs.Scatter3d(
x=x_edge, y=y_edge, z=z_edge, mode='lines',
line=graph_objs.Line(color='rgb(50, 50, 50)',
width=1.5)
line=graph_objs.Line(
color=edges_color,
width=1.5
),
showlegend=False
)
if show_colorbar is True:

if not isinstance(mean_dists[0], str) and show_colorbar is True:
return graph_objs.Data([triangles, lines, colorbar])
else:
return graph_objs.Data([triangles, lines])
Expand All @@ -3407,6 +3420,7 @@ def create_trisurf(x, y, z, simplices, colormap=None, show_colorbar=True,
backgroundcolor='rgb(230, 230, 230)',
gridcolor='rgb(255, 255, 255)',
zerolinecolor='rgb(255, 255, 255)',
edges_color='rgb(50, 50, 50)',
height=800, width=800,
aspectratio=dict(x=1, y=1, z=1)):
"""
Expand Down Expand Up @@ -3440,6 +3454,7 @@ def create_trisurf(x, y, z, simplices, colormap=None, show_colorbar=True,
inclusive
:param (str) zerolinecolor: color of the axes. Takes a string of the
form 'rgb(x,y,z)' x,y,z are between 0 and 255 inclusive
:param (str) edges_color: color of the edges, if plot_edges is True
:param (int|float) height: the height of the plot (in pixels)
:param (int|float) width: the width of the plot (in pixels)
:param (dict) aspectratio: a dictionary of the aspect ratio values for
Expand Down Expand Up @@ -3622,6 +3637,7 @@ def dist_origin(x, y, z):
x, y, z, simplices,
color_func=colors,
show_colorbar=True,
edges_color='rgb(2,85,180)',
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason not to be consistent about the spacing? rgb(2, 85, 180)?

title=' Modern Art'
)

Expand All @@ -3637,6 +3653,7 @@ def dist_origin(x, y, z):
show_colorbar=show_colorbar,
color_func=color_func,
colormap=colormap,
edges_color=edges_color,
plot_edges=plot_edges)
axis = dict(
showbackground=showbackground,
Expand Down