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

minor test fixes for ipyplotly_integration #975

Merged
merged 145 commits into from
Apr 18, 2018
Merged
Show file tree
Hide file tree
Changes from 42 commits
Commits
Show all changes
145 commits
Select commit Hold shift + click to select a range
77b50f3
Update CHANGELOG.md
Kully Feb 18, 2018
fafcabf
(matplotlylib) Make convert_dash more robust to changes in matplotlib.
Feb 18, 2018
cbf037d
commit
Kully Feb 19, 2018
065e59c
remove unnecessary data files in tests and robust data-path searching
Kully Feb 19, 2018
9502440
pep-8: l -> L variable name
Kully Feb 19, 2018
f2ba912
bump version number up
Kully Feb 19, 2018
a15b744
Merge pull request #939 from plotly/rewrite-changelog-choro
Kully Feb 19, 2018
5818d2b
remove shapefiles from folders and delete folders
Kully Feb 20, 2018
6a7e7f2
removed data/ folder and modified choropleth code to match file names
Kully Feb 20, 2018
6319d63
Merge pull request #940 from plotly/fix-choropleth
Kully Feb 20, 2018
c7bff98
changelog for fixed 2.4.1
Kully Feb 21, 2018
3d8cd07
chelsea's comments
Kully Feb 21, 2018
cb71b64
Merge pull request #943 from plotly/changelog-for-fixed-choropleth
Kully Feb 21, 2018
1b09e21
added fill_percent to params for insert, swap and remove
Kully Feb 23, 2018
b149e0d
html preview not working
Kully Feb 26, 2018
9705dde
HTML preview now reflects the fill_percent param settings
Kully Feb 26, 2018
c26b763
tests are working
Kully Feb 26, 2018
38f88ef
make update_default_schema
Kully Feb 26, 2018
1991c7f
udpated changelog, removed TODOs, PEP-8 79 char line limit
Kully Feb 27, 2018
35716d6
Merge branch 'master' into dashboard-percent
Kully Feb 27, 2018
004bbb0
turn off hover for the lines
Kully Feb 28, 2018
c3682d3
0-padding
Kully Feb 28, 2018
8f557c7
changelog
Kully Feb 28, 2018
a85cb8b
update changelog and version number for 2.5.0
Kully Feb 28, 2018
8517f3f
change fill_percent example in doc to fill_percent=20
Kully Feb 28, 2018
3c0d337
sort all node generators to make HTML output consistent for py2 + py3
Kully Feb 28, 2018
8def60c
add choropleth files back
Kully Feb 28, 2018
524a8a0
Merge pull request #938 from nburrus/convert-dash
jackparmer Mar 1, 2018
b091aaf
dashboards now autosize based on GUI
Kully Mar 2, 2018
14a6a02
update dash workshop announcement
cldougl Mar 2, 2018
e994e55
Merge pull request #953 from plotly/announcement_update
cldougl Mar 5, 2018
85d7549
version num to 2.5.0
Kully Mar 5, 2018
f898892
4 equal height vertical stacked example in doc string
Kully Mar 5, 2018
2c07b12
rename box_1 to box_a, etc
Kully Mar 5, 2018
736c42d
remove shapefiles in tests dir
Kully Mar 5, 2018
7f08843
change version back for testing purposes
Kully Mar 5, 2018
97213c9
version to 2.5.0
Kully Mar 5, 2018
0482dce
try bumping npm v in circle.yml to 6.0.0
Kully Mar 6, 2018
edbd796
Don't crash if figure_factory is imported but pandas isn't installed.…
dpryan79 Mar 6, 2018
8e7c112
Merge pull request #958 from dpryan79/fix941
Kully Mar 6, 2018
c43c436
pr for #958 PR
Kully Mar 6, 2018
e406bef
add PR reference
Kully Mar 6, 2018
fbaaba2
remove a comment
Kully Mar 7, 2018
8c7eb17
Merge pull request #947 from plotly/offline_mode_choropleth
Kully Mar 7, 2018
8bde810
Merge branch 'master' into dashboard-percent
Kully Mar 7, 2018
d6d777b
Merge pull request #946 from plotly/dashboard-percent
Kully Mar 7, 2018
56de411
Merge branch 'master' into changelog-ff-crash
Kully Mar 8, 2018
98feab6
Merge pull request #959 from plotly/changelog-ff-crash
Kully Mar 8, 2018
cbc1779
update plotlyjs for offline
cldougl Mar 12, 2018
2f59570
Merge pull request #971 from plotly/bump_pjs_1352
cldougl Mar 12, 2018
70f82c4
moved py2.7 test env to last in core environments
Kully Mar 15, 2018
1d4f440
remove ipyplotly folder
Kully Mar 15, 2018
f39a85b
fixed assert Scatter() == dict(type='scatter')
Kully Mar 15, 2018
f3f6811
fixed test_access_top_level in test_figure
Kully Mar 15, 2018
07c0977
more in test_figures fixes
Kully Mar 15, 2018
36ed3aa
merged to ipyplotly integration
Kully Mar 16, 2018
425c36a
remove junk in test_figure.py from before base merge
Kully Mar 16, 2018
b2f5064
BaseFigure and BasePlotlyType inherit from PlotlyBase class
Kully Mar 16, 2018
de95849
remove commented validation function
Kully Mar 16, 2018
c611a23
comment out validate error in test_scatter
Kully Mar 19, 2018
e714499
add histogram2dcontour back to graph_reference TRACES
Kully Mar 19, 2018
9ce82b4
FigureWidget to OLD_CLASS_NAMES
Kully Mar 19, 2018
234b979
add update to Data def in graph_objs//remove to_string in test_update
Kully Mar 19, 2018
55969d9
fixed test_offline in optional
Kully Mar 20, 2018
6f822cd
test_plotl_mpl fixed
Kully Mar 20, 2018
bb90864
add py36 to core and optional test envs
Kully Mar 21, 2018
41a63ff
break up datetime tests into multiple ones for better test control
Kully Mar 21, 2018
871341c
fixed JSONEncoding errors
Kully Mar 22, 2018
7a9aa99
last assert test in test_utils
Kully Mar 22, 2018
d2ca807
print statements for testing xaxis1
Kully Mar 23, 2018
de5d242
construct paths to choropleth files intelligently
Kully Mar 26, 2018
183a6fa
update the changelog
Kully Mar 26, 2018
53d57ca
bump version
Kully Mar 26, 2018
2260054
changelog comments
Kully Mar 26, 2018
0bde4e1
Merge pull request #980 from plotly/windows-choropleth
Kully Mar 26, 2018
4a977cd
merge from Jon's PR
Kully Mar 27, 2018
c154dd6
merge from ipyplotly_integration
Kully Mar 27, 2018
f332e29
revert to current basedatatypes from #942
Kully Mar 28, 2018
4397815
write in helper function for assert_dict_equal
Kully Mar 28, 2018
6988d08
fix trisurf: hoverinfo needs 'none', not 'None'
Kully Mar 28, 2018
0ea9e14
finished a few more classes in figure_factory tests
Kully Mar 28, 2018
0f04191
remove comment
Kully Mar 29, 2018
274b003
change error msg for pip installs in choropleth
Kully Mar 29, 2018
f982427
changelog update
Kully Mar 29, 2018
77cf63f
fixed facet grid - annotations now tuples - FacetGrid tests in test_o…
Kully Mar 29, 2018
588f7dd
remove DeprecationWarning from _dendrogram.py
Kully Mar 29, 2018
24ef193
remove unneeded comments in facet_grid
Kully Mar 29, 2018
8386272
change height, width default to np.inf in _Dendogram
Kully Mar 29, 2018
b256334
fixed all tests in optional figure_factory
Kully Mar 29, 2018
b8d04a6
PEP8 to greyscale list
Kully Mar 29, 2018
e890a4a
fix Quiver tests; remove commented TestDisplot for duplicate
Kully Mar 31, 2018
100ef19
minor test fixes in test_tools resolved
Kully Apr 2, 2018
89c4d38
playing around with matplotlylib - no success
Kully Apr 2, 2018
51cd9bc
chelsea's comment
Kully Apr 2, 2018
9f85771
fixed test_validate tests in test_core
Kully Apr 3, 2018
c4d1795
fixed offline tests
Kully Apr 3, 2018
cff0312
change assert error in decorator
Kully Apr 3, 2018
399c2dc
flawed annotations dont return error, so not checking for one
Kully Apr 3, 2018
98e2f54
fix minor tests in test_data
Kully Apr 3, 2018
330d99b
fix test_error_bars
Kully Apr 3, 2018
760b36a
null changes
Kully Apr 3, 2018
9ffc488
added white space
Kully Apr 4, 2018
0d96800
Merge branch 'ipyplotly_integration' into rebased-test-fixes
Kully Apr 4, 2018
cbd1304
rework mock import for Py3.3+ compat
Kully Apr 4, 2018
efd4c2a
deepcopy in assert_fig_equal
Kully Apr 4, 2018
f87a8b8
fixed all but one test in make_subplots
Kully Apr 4, 2018
924501b
merge from ipyplotly-branch
Kully Apr 5, 2018
c90fe9c
Merge branch 'rebased-test-fixes' of https://github.com/plotly/plotly…
Kully Apr 5, 2018
0bf48c2
fixed test_api - mock compatib issue
Kully Apr 5, 2018
8fd5985
change errortype to ValueError to fix test
Kully Apr 5, 2018
a4d6c5d
fix merge for rebase continue
Kully Apr 5, 2018
0505070
reverted back to old test in test_figure
Kully Apr 5, 2018
c937f8e
Merge branch 'ipyplotly_integration' of https://github.com/plotly/plo…
Kully Apr 5, 2018
3e53c10
merge to ipyplotly
Kully Apr 5, 2018
7a0ad41
Merge pull request #984 from plotly/better-geopandas-msg
Kully Apr 5, 2018
d93772d
validators update
Kully Apr 5, 2018
1d5db95
Merge branch 'ipyplotly_integration' of https://github.com/plotly/plo…
Kully Apr 6, 2018
56d0e5f
merging from ipyplotly_integration for up-to-date rebase
Kully Apr 6, 2018
46c9186
fix test_graph_objs.py in test_core
Kully Apr 6, 2018
3140f29
fix scatter.py
Kully Apr 6, 2018
7fa119a
fix self.assert_fig_equal(ohlc.to_plotly_json()['data'][0],
Kully Apr 6, 2018
a3e3508
assert_dict_equal typo
Kully Apr 6, 2018
3aaa3ed
fix PotlyError -> ValueError for TestTable class
Kully Apr 6, 2018
f6f7aaf
middle of putting .to_plotly_json in assert_fig_equal
Kully Apr 6, 2018
35bf539
Merge branch 'master' of https://github.com/plotly/plotly.py into ipy…
Kully Apr 12, 2018
06fec5c
Merge branch 'ipyplotly_integration' of https://github.com/plotly/plo…
Kully Apr 12, 2018
0e53bd0
Merge branch 'ipyplotly_integration' into rebased-test-fixes
Kully Apr 12, 2018
a59c0c2
updated test_tools - numpy arrays to lists
Kully Apr 12, 2018
8f1e212
test_datetimes fixes
Kully Apr 12, 2018
365edce
Fix typo (agruments)
simon04 Apr 13, 2018
e932378
Merge pull request #989 from simon04/agruments
Kully Apr 13, 2018
b981e72
test_offline changes
Kully Apr 13, 2018
23646ca
do not strip fig of invalid elements wen running .get_figure\nthis is…
Kully Apr 13, 2018
5740d73
pull from maste
Kully Apr 16, 2018
eeb7f7e
merge from ipyplotly_integration
Kully Apr 16, 2018
ec5cc2c
xaxis -> xaxis1 in make_subplots
Kully Apr 16, 2018
a561275
Merge branch 'ipyplotly_integration' of https://github.com/plotly/plo…
Kully Apr 16, 2018
02ad817
Merge branch 'ipyplotly_integration' into rebased-test-fixes
Kully Apr 16, 2018
2d9cc9e
xaxis -> xaxis1
Kully Apr 17, 2018
41b0d03
test_plot
Kully Apr 17, 2018
46051e9
fixed test_figure
Kully Apr 17, 2018
499f29f
fixed test_append_trace - added assert_fig_equal func clone in test_core
Kully Apr 17, 2018
de4e6c6
fixed TestDistplot tests
Kully Apr 17, 2018
5b0d607
removed test_plotly_base_classes test as PlotlyDict/PlotlyList are re…
Kully Apr 17, 2018
42e2462
fixed tests in test_ff in optional
Kully Apr 17, 2018
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
1 change: 0 additions & 1 deletion plotly/basedatatypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -2058,4 +2058,3 @@ def __init__(self, plotly_name, **kwargs):
def _send_update(self, prop, val):
# Frames are not supported by FrameWidget and updates are not propagated to parents
pass

9 changes: 5 additions & 4 deletions plotly/figure_factory/_dendrogram.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,15 @@ def create_dendrogram(X, orientation="bottom", labels=None,
distfun=distfun, linkagefun=linkagefun,
hovertext=hovertext)

return graph_objs.Figure(data=dendrogram.data, layout=dendrogram.layout)
return graph_objs.Figure(data=dendrogram.data,
layout=dendrogram.layout)


class _Dendrogram(object):
"""Refer to FigureFactory.create_dendrogram() for docstring."""

def __init__(self, X, orientation='bottom', labels=None, colorscale=None,
width="100%", height="100%", xaxis='xaxis', yaxis='yaxis',
width=np.inf, height=np.inf, xaxis='xaxis', yaxis='yaxis',
Copy link
Member

Choose a reason for hiding this comment

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

this seems odd to me: infinite width?

Copy link
Contributor

Choose a reason for hiding this comment

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

Context: The string '100%' is forbidden by the number valType of width.

Copy link
Member

Choose a reason for hiding this comment

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

Got it. I wonder what width="100%" was doing before. I don't think that it doesn't anything in plotly.js as width='100%' and height='100%' should correspond to autosize=True.

Maybe the better setting would be to:

  • set the arguments as width=None and height=None
  • if width or height is not None, apply it in the figure
  • set autosize=True

distfun=None,
linkagefun=lambda x: sch.linkage(x, 'complete'),
hovertext=None):
Expand Down Expand Up @@ -126,7 +127,7 @@ def __init__(self, X, orientation='bottom', labels=None, colorscale=None,
(dd_traces, xvals, yvals,
ordered_labels, leaves) = self.get_dendrogram_traces(X, colorscale,
distfun,
linkagefun,
linkagefun,
hovertext)

self.labels = ordered_labels
Expand Down Expand Up @@ -291,7 +292,7 @@ def get_dendrogram_traces(self, X, colorscale, distfun, linkagefun, hovertext):
x=np.multiply(self.sign[self.xaxis], xs),
y=np.multiply(self.sign[self.yaxis], ys),
mode='lines',
marker=graph_objs.Marker(color=colors[color_key]),
marker=graph_objs.scatter.Marker(color=colors[color_key]),
text=hovertext_label,
hoverinfo='text'
)
Expand Down
42 changes: 18 additions & 24 deletions plotly/figure_factory/_facet_grid.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,9 @@

from plotly import colors, exceptions, optional_imports
from plotly.figure_factory import utils
from plotly.graph_objs import graph_objs
from plotly.tools import make_subplots

import math
import copy
from numbers import Number

pd = optional_imports.get_module('pandas')
Expand Down Expand Up @@ -108,7 +106,7 @@ def _annotation_dict(text, lane, num_of_lanes, SUBPLOT_SPACING, row_col='col',
showarrow=False,
xref='paper',
yref='paper',
text=text,
text=str(text),
Copy link
Member

Choose a reason for hiding this comment

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

why was this change necessary?

Copy link
Contributor

Choose a reason for hiding this comment

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

Context: layout.annotation.text has a string valType. I don't currently autoconvert numbers (or any other Python types) to strings.

Do we want to support any string coercions?

Copy link
Member

Choose a reason for hiding this comment

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

This seems like it should be an addition to the plotly.js schema. I'll make an issue

Copy link
Member

Choose a reason for hiding this comment

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

Tracking in plotly/plotly.js#2523

font=dict(
size=13,
color=AXIS_TITLE_COLOR
Expand Down Expand Up @@ -331,10 +329,7 @@ def _facet_grid_color_categorical(df, x, y, facet_row, facet_col, color_name,
row_col='row', flipped=flipped_rows)
)

# add annotations
fig['layout']['annotations'] = annotations

return fig
return fig, annotations
Copy link
Member

Choose a reason for hiding this comment

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

why was this change made? was it for this? https://github.com/plotly/plotly.py/pull/975/files#diff-83b399f13d8b361d38016264ed1ce589R987 If so, why does fig['layout']['annotations'].append not work anymore?

Copy link
Contributor

Choose a reason for hiding this comment

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

Context: Object array properties like (e.g. layout.annotations, layout.images, etc.) are now stored as tuples instead of lists so that they are immutable.
So fig['layout']['annotations'].append(annot) won't work anymore, but fig['layout']['annotations'] += (annot,) will work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks Jon for jumping in on Chris' comments. You definitely know the code changes better than I do.

@chriddyp I reworked facet_grid to accommodate annotations being tuples and not lists

Copy link
Member

Choose a reason for hiding this comment

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

I see. So if I do

fig = go.Figure(layout=go.Layout(annotations=[go.Annotation(text='...')]))

then annotations will get cast as a tuple.

Marking this one as a 🚨 - Breaking Change



def _facet_grid_color_numerical(df, x, y, facet_row, facet_col, color_name,
Expand Down Expand Up @@ -474,10 +469,7 @@ def _facet_grid_color_numerical(df, x, y, facet_row, facet_col, color_name,
row_col='row', flipped=flipped_rows)
)

# add annotations
fig['layout']['annotations'] = annotations

return fig
return fig, annotations


def _facet_grid(df, x, y, facet_row, facet_col, num_of_rows,
Expand Down Expand Up @@ -600,10 +592,7 @@ def _facet_grid(df, x, y, facet_row, facet_col, num_of_rows,
row_col='row', flipped=flipped_rows)
)

# add annotations
fig['layout']['annotations'] = annotations

return fig
return fig, annotations


def create_facet_grid(df, x=None, y=None, facet_row=None, facet_col=None,
Expand Down Expand Up @@ -905,7 +894,7 @@ def create_facet_grid(df, x=None, y=None, facet_row=None, facet_col=None,
j = 0
colormap[val] = default_colors[j]
j += 1
fig = _facet_grid_color_categorical(
fig, annotations = _facet_grid_color_categorical(
df, x, y, facet_row, facet_col, color_name, colormap,
num_of_rows, num_of_cols, facet_row_labels, facet_col_labels,
trace_type, flipped_rows, flipped_cols, show_boxes,
Expand All @@ -924,7 +913,7 @@ def create_facet_grid(df, x=None, y=None, facet_row=None, facet_col=None,
"all the values of the colormap column are in "
"the keys of your dictionary."
)
fig = _facet_grid_color_categorical(
fig, annotations = _facet_grid_color_categorical(
df, x, y, facet_row, facet_col, color_name, colormap,
num_of_rows, num_of_cols, facet_row_labels,
facet_col_labels, trace_type, flipped_rows,
Expand All @@ -936,7 +925,7 @@ def create_facet_grid(df, x=None, y=None, facet_row=None, facet_col=None,
colorscale_list = colormap
utils.validate_colorscale(colorscale_list)

fig = _facet_grid_color_numerical(
fig, annotations = _facet_grid_color_numerical(
df, x, y, facet_row, facet_col, color_name,
colorscale_list, num_of_rows, num_of_cols,
facet_row_labels, facet_col_labels, trace_type,
Expand All @@ -952,7 +941,7 @@ def create_facet_grid(df, x=None, y=None, facet_row=None, facet_col=None,
"of a Plotly Colorscale. The available colorscale "
"names are {}".format(colors.PLOTLY_SCALES.keys())
)
fig = _facet_grid_color_numerical(
fig, annotations = _facet_grid_color_numerical(
df, x, y, facet_row, facet_col, color_name,
colorscale_list, num_of_rows, num_of_cols,
facet_row_labels, facet_col_labels, trace_type,
Expand All @@ -961,7 +950,7 @@ def create_facet_grid(df, x=None, y=None, facet_row=None, facet_col=None,
)
else:
colorscale_list = colors.PLOTLY_SCALES['Reds']
fig = _facet_grid_color_numerical(
fig, annotations = _facet_grid_color_numerical(
df, x, y, facet_row, facet_col, color_name,
colorscale_list, num_of_rows, num_of_cols,
facet_row_labels, facet_col_labels, trace_type,
Expand All @@ -970,7 +959,7 @@ def create_facet_grid(df, x=None, y=None, facet_row=None, facet_col=None,
)

else:
fig = _facet_grid(
fig, annotations = _facet_grid(
df, x, y, facet_row, facet_col, num_of_rows, num_of_cols,
facet_row_labels, facet_col_labels, trace_type, flipped_rows,
flipped_cols, show_boxes, SUBPLOT_SPACING, marker_color,
Expand All @@ -992,8 +981,10 @@ def create_facet_grid(df, x=None, y=None, facet_row=None, facet_col=None,
# axis titles
x_title_annot = _axis_title_annotation(x, 'x')
y_title_annot = _axis_title_annotation(y, 'y')
fig['layout']['annotations'].append(x_title_annot)
fig['layout']['annotations'].append(y_title_annot)

# annotations
annotations.append(x_title_annot)
annotations.append(y_title_annot)

# legend
fig['layout']['showlegend'] = show_legend
Expand All @@ -1008,9 +999,12 @@ def create_facet_grid(df, x=None, y=None, facet_row=None, facet_col=None,
if ggplot2:
if color_name:
legend_annot = _legend_annotation(color_name)
fig['layout']['annotations'].append(legend_annot)
annotations.append(legend_annot)
fig['layout']['margin']['r'] = 150

# assign annotations to figure
fig['layout']['annotations'] = annotations

# add shaded boxes behind axis titles
if show_boxes and ggplot2:
_add_shapes_to_fig(fig, ANNOT_RECT_COLOR, flipped_rows, flipped_cols)
Expand Down
2 changes: 1 addition & 1 deletion plotly/figure_factory/_trisurf.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ def trisurf(x, y, z, simplices, show_colorbar, edges_color, scale,
color=[min_mean_dists, max_mean_dists],
colorscale=colorscale,
showscale=True),
hoverinfo='None',
hoverinfo='none',
Copy link
Member

Choose a reason for hiding this comment

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

Why was this change made?
On another note, the fact that this was 'None' before seems a little odd to me: why not just None?

Copy link
Contributor

Choose a reason for hiding this comment

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

Context: here is the schema for scatter3d.hoverinfo

            "hoverinfo": {
                    "arrayOk": true,
                    "description": "Determines which trace information appear on hover. If `none` or `skip` are set, no information is displayed upon hovering. But, if `none` is set, click and hover events are still fired.",
                    "dflt": "all",
                    "editType": "calc",
                    "extras": [
                        "all",
                        "none",
                        "skip"
                    ],
                    "flags": [
                        "x",
                        "y",
                        "z",
                        "text",
                        "name"
                    ],
                    "role": "info",
                    "valType": "flaglist"
                },

The string 'none' is one of the valid extras states. Currently, I require all flaglist values to be a case-sensitive match to those in the schema. Setting hoverinfo to the Python None would mean to unset any previously set value and let plotly.js choose the default (all in this case).

Should we be more generous and coerce case-insensitive matches to the form in the schema?

Copy link
Member

Choose a reason for hiding this comment

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

Ah I see. I wonder if plotly.js accepted None before. I don't think that we should coerce the case-insensitive matches, this seems good to me.

Nice to see that this new validation is improving our own code 😉

showlegend=False
)

Expand Down
1 change: 1 addition & 0 deletions plotly/graph_objs/graph_objs.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
from plotly.graph_objs import *
Copy link
Member

Choose a reason for hiding this comment

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

this change seems odd to me, seems like something that should exist (if necessary) in #942. What's the rational behind this change?

Copy link
Contributor

Choose a reason for hiding this comment

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

I actually did add this in #942 as well. It is a bit weird, but some tests (and examples I've seen online) have something like from plotly.graph_objs import graph_objs (e.g. https://plot.ly/python/scatterplot-matrix/)

It's an easy enough thing to keep backward compatible, but do we want to keep it around or deprecate it?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that's odd. It seems like the more semantic way to write this would be

import plotly.graph_objs as graph_objs

or, the way we've been doing it all along

import plotly.graph_objs as go

In those examples (https://plot.ly/python/scatterplot-matrix/) graph_objs isn't even used.

OK, well let's keep it for backwards compatibility but I'd be OK with deprecating it.

2 changes: 2 additions & 0 deletions plotly/graph_reference.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
'Histogram2d': {'object_name': 'histogram2d', 'base_type': dict},
'Histogram2dContour': {'object_name': 'histogram2dcontour',
'base_type': dict},
'Histogram2dcontour': {'object_name': 'histogram2dcontour',
'base_type': dict},
Copy link
Member

Choose a reason for hiding this comment

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

this change seems odd to me, seems like something that should exist (if necessary) in #942. What's the rational behind this change?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should be able to delete graph_reference.py altogether (but maybe it's still used for testing?). I copied this mapping to codegen/compatibility.py to use when generating the deprecated classes.

Copy link
Member

Choose a reason for hiding this comment

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

OK yeah, this seems like something we can delete. I forgot all about graph_reference - this is from the old days when we didn't have a plotly.s plot-schema!

@Kully - Can you chime in here? Do we still need graph_reference.py?

'Layout': {'object_name': 'layout', 'base_type': dict},
'Legend': {'object_name': 'legend', 'base_type': dict},
'Line': {'object_name': 'line', 'base_type': dict},
Expand Down
18 changes: 6 additions & 12 deletions plotly/tests/test_core/test_graph_objs/test_annotations.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
from plotly.graph_objs import Annotation, Annotations, Data, Figure, Layout



def setup():
import warnings
warnings.filterwarnings('ignore')
Expand All @@ -25,33 +24,28 @@ def test_trivial():
assert Annotations() == list()


@raises(PlotlyError)
def test_weird_instantiation(): # Python allows this, but nonsensical for us.
print(Annotations({}))
assert Annotations({}) == list()
Copy link
Member

Choose a reason for hiding this comment

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

🚨 - Breaking Change
(Logging this so that we can document all of the breaking changes later)



def test_dict_instantiation():
Annotations([{'text': 'annotation text'}])


@raises(PlotlyDictKeyError)
def test_dict_instantiation_key_error():
print(Annotations([{'not-a-key': 'anything'}]))
assert Annotations([{'not-a-key': 'anything'}]) == [{'not-a-key': 'anything'}]
Copy link
Member

Choose a reason for hiding this comment

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

🚨 - Breaking Change

However, this one seems like the proper behavior. How do we instantiate a list of dictionaries with the new graph_objs?

Copy link
Member

Choose a reason for hiding this comment

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

OK I see, the new way to do this would be to just use a regular list of go.layout.Annotation:

[go.layout.Annotation(**{'not-a-key': 'anything'})]

Copy link
Member

Choose a reason for hiding this comment

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

So, we should probably update this test name as well (we can do that in the end, once we've gone through the rest of the tests)



@raises(PlotlyDictValueError)
def test_dict_instantiation_key_error():
print(Annotations([{'font': 'not-a-dict'}]))
def test_dict_instantiation_key_error_2():
assert Annotations([{'font': 'not-a-dict'}]) == [{'font': 'not-a-dict'}]


@raises(PlotlyListEntryError)
def test_dict_instantiation_graph_obj_error_0():
Annotations([Data()])
assert Annotations([Data()]) == [[]]


@raises(PlotlyListEntryError)
def test_dict_instantiation_graph_obj_error_2():
Annotations([Annotations()])
assert Annotations([Annotations()]) == [[]]


def test_validate():
Expand Down
28 changes: 14 additions & 14 deletions plotly/tests/test_core/test_graph_objs/test_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,42 +25,42 @@ def test_trivial():
assert Data() == list()


@raises(PlotlyError)
#@raises(PlotlyError)
def test_weird_instantiation(): # Python allows this...
print(Data({}))
assert Data({}) == []
Copy link
Member

Choose a reason for hiding this comment

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

🚨 - Breaking Change



def test_default_scatter():
assert Data([{}]) == list([{'type': 'scatter'}])
assert Data([{}]) == list([{}])
Copy link
Member

Choose a reason for hiding this comment

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

🚨 - Breaking Change



def test_dict_instantiation():
Data([{'type': 'scatter'}])


@raises(PlotlyDictKeyError)
# @raises(PlotlyDictKeyError)
def test_dict_instantiation_key_error():
print(Data([{'not-a-key': 'anything'}]))
assert Data([{'not-a-key': 'anything'}]) == [{'not-a-key': 'anything'}]
Copy link
Member

Choose a reason for hiding this comment

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

🚨 - Breaking Change



@raises(PlotlyDictValueError)
def test_dict_instantiation_key_error():
print(Data([{'marker': 'not-a-dict'}]))
# @raises(PlotlyDictValueError)
def test_dict_instantiation_key_error_2():
assert Data([{'marker': 'not-a-dict'}]) == [{'marker': 'not-a-dict'}]


@raises(PlotlyDataTypeError)
# @raises(PlotlyDataTypeError)
def test_dict_instantiation_type_error():
Data([{'type': 'invalid_type'}])
assert Data([{'type': 'invalid_type'}]) == [{'type': 'invalid_type'}]


@raises(PlotlyListEntryError)
# @raises(PlotlyListEntryError)
def test_dict_instantiation_graph_obj_error_0():
Data([Data()])
assert Data([Data()]) == [[]]


@raises(PlotlyListEntryError)
# raises(PlotlyListEntryError)
def test_dict_instantiation_graph_obj_error_2():
Data([Annotations()])
assert Data([Annotations()]) == [[]]


def test_validate():
Expand Down
3 changes: 1 addition & 2 deletions plotly/tests/test_core/test_graph_objs/test_error_bars.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,5 @@ def test_instantiate_error_y():
width=5)


@raises(PlotlyDictKeyError)
def test_key_error():
ErrorX(value=0.1, typ='percent', color='red')
assert ErrorX(value=0.1, typ='percent', color='red') == {'color': 'red', 'typ': 'percent', 'value': 0.1}
Copy link
Member

Choose a reason for hiding this comment

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

So, if we're deprecating go.ErrorX in favor of go.scatter.ErrorX, then we should probably have two tests here:

  • One for the deprecated behaviour (errors are no longer thrown)
  • Another for the new syntax, which should basically be the same test as before:
@raises(PlotlyDictKeyError)
def test_key_error():
    go.scatter.ErrorX(value=0.1, typ='percent', color='red')

And we should verify that the error that is raised remains PlotlyDictKeyError

Copy link
Member

Choose a reason for hiding this comment

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

🚨 - Breaking Change

2 changes: 1 addition & 1 deletion plotly/tests/test_core/test_graph_objs/test_figure.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def test_instantiation(self):
'frames': []
}

Figure(native_figure)
Figure(**native_figure)
Copy link
Member

Choose a reason for hiding this comment

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

🚨 - Breaking Change

Copy link
Contributor

Choose a reason for hiding this comment

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

FYI: Figure can now take in another figure, or a graph representing a figure, as the first arg.

Copy link
Member

Choose a reason for hiding this comment

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

OK sounds good, let's revert back to the previous test then (cc @Kully )

Figure()

def test_access_top_level(self):
Expand Down
33 changes: 26 additions & 7 deletions plotly/tests/test_core/test_graph_objs/test_frames.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
from unittest import TestCase

from plotly import exceptions
from plotly.graph_objs import Bar, Frames
from plotly.graph_objs import Bar, Frames, Frame

import re


class FramesTest(TestCase):
Expand Down Expand Up @@ -56,20 +58,37 @@ def test_deeply_nested_layout_attributes(self):
)

def test_deeply_nested_data_attributes(self):
frames = Frames()
frames.append({})
frames[0].data = [Bar()]
frames[0].data[0].marker.color = 'red'
#frames = Frames()
#frames.append({})
#frames[0].data = [Bar()]
#frames[0].data[0].marker.color = 'red'

frames = Frame
frames.data = [Bar()]
frames.data[0].marker.color = 'red'
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't seem right to me. The new syntax is replacing this with a list of Frame objects,

help(go.Frames)
        plotly.graph_objs.Frames is deprecated.
Please replace it with a list or tuple of instances of the following types
  - plotly.graph_objs.Frame

So, this test should be:

frames = []
frames.append(go.Frame())
frames[0].data = [go.Bar()]
frames[0].data[0].marker.color = 'red'

Copy link
Member

Choose a reason for hiding this comment

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

Although go.Frame() doesn't seem to work:

In [11]: go.Frame()
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-11-d2b9d2e3e8b5> in <module>()
----> 1 go.Frame()

~/Repos/plotlypy-jmease/plotly.py/plotly/graph_objs/_frame.py in __init__(self, baseframe, data, group, layout, name, traces, **kwargs)
    181         # ---------------------
    182         self._validators['baseframe'] = v_frame.BaseframeValidator()
--> 183         self._validators['data'] = v_frame.DataValidator()
    184         self._validators['group'] = v_frame.GroupValidator()
    185         self._validators['layout'] = v_frame.LayoutValidator()

~/Repos/plotlypy-jmease/plotly.py/plotly/validators/frame/_data.py in __init__(self, plotly_name, parent_name)
      6     def __init__(self, plotly_name='data', parent_name='frame'):
      7         super().__init__(
----> 8             plotly_name=plotly_name, parent_name=parent_name, role='object'
      9         )

TypeError: __init__() got an unexpected keyword argument 'role'

Am I doing something wrong here? cc @jmmease

Copy link
Contributor

Choose a reason for hiding this comment

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

Nope, I just broke it :-) Fixed in 8a7b476


# parse out valid attrs from ._prop_descriptions
prop_descrip = frames.data[0].marker.line._prop_descriptions
prop_descrip

raw_matches = re.findall(
"\n [a-z]+| [a-z]+\n", prop_descrip
)
matches = []
for r in raw_matches:
r = r.replace(' ', '')
r = r.replace('\n', '')
matches.append(r)

# It's OK if this needs to change, but we should check *something*.
self.assertEqual(
frames[0].data[0].marker.line._get_valid_attributes(),
set(matches),
Copy link
Member

@chriddyp chriddyp Apr 3, 2018

Choose a reason for hiding this comment

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

Instead of a regex, a better way to do this might be to assert on

all_attributes = dir(frames[0].data[0].marker.line)
public_attributes = [d for d in all_attributes if d[0] != '_' and d[:1] != 'on']
assertEqual(set(public_attributes), {'cliponaxis',
 'connectgaps',
 'customdata',
 'customdatasrc',
 'dx',
 'dy',
 'error_x',
 'error_y',
 'figure',
 'fill',
 'fillcolor',
 'hoverinfo',
 'hoverinfosrc',
 'hoverlabel',
 'hoveron',
 'hovertext',
 'hovertextsrc',
 'ids',
 'idssrc',
 'legendgroup',
 'line',
 'marker',
 'mode',
 'name',
 'opacity',
 'parent',
 'plotly_name',
 'r',
 'rsrc',
 'selected',
 'selectedpoints',
 'showlegend',
 'stream',
 't',
 'text',
 'textfont',
 'textposition',
 'textpositionsrc',
 'textsrc',
 'to_plotly_json',
 'tsrc',
 'type',
 'uid',
 'unselected',
 'update',
 'visible',
 'x',
 'x0',
 'xaxis',
 'xcalendar',
 'xsrc',
 'y',
 'y0',
 'yaxis',
 'ycalendar',
 'ysrc'})

{'colorsrc', 'autocolorscale', 'cmin', 'colorscale', 'color',
'reversescale', 'width', 'cauto', 'widthsrc', 'cmax'}
)

def test_frame_only_attrs(self):
frames = Frames()
frames = Frame
Copy link
Member

Choose a reason for hiding this comment

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

frames.append({})

# It's OK if this needs to change, but we should check *something*.
Expand Down
7 changes: 4 additions & 3 deletions plotly/tests/test_core/test_graph_objs/test_graph_objs.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@
import plotly.graph_objs as go
import plotly.graph_reference as gr

# added FigureWidget to OLD_CLASS_NAMES (v 2 and lower)
OLD_CLASS_NAMES = ['AngularAxis', 'Annotation', 'Annotations', 'Area',
'Bar', 'Box', 'ColorBar', 'Contour', 'Contours',
'Data', 'ErrorX', 'ErrorY', 'ErrorZ', 'Figure',
'Font', 'Frames', 'Heatmap', 'Histogram', 'Histogram2d',
'Histogram2dContour', 'Layout', 'Legend', 'Line',
'Margin', 'Marker', 'RadialAxis', 'Scatter',
'FigureWidget', 'Font', 'Frames', 'Heatmap', 'Histogram',
'Histogram2d', 'Histogram2dContour', 'Layout', 'Legend',
'Line', 'Margin', 'Marker', 'RadialAxis', 'Scatter',
'Scatter3d', 'Scene', 'Stream', 'Surface', 'Trace',
'XAxis', 'XBins', 'YAxis', 'YBins', 'ZAxis']

Expand Down
Loading