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

Enabled warnings about the use of __call__ on travis #1779

Merged
merged 9 commits into from
Aug 3, 2017

Conversation

jlstevens
Copy link
Contributor

This change should help us detect uses of __call__ for styling (now deprecated).

@jlstevens
Copy link
Contributor Author

Issue #1758 will also be helpful to find remaining uses of __call__.

@jlstevens
Copy link
Contributor Author

jlstevens commented Aug 3, 2017

I expect Travis to flag these uses (found in #1758):

/doc/Tutorials/Bokeh_Backend.ipynb:100:    "(hv.Curve(data)(style=curve_opts) +\n",
./doc/Tutorials/Bokeh_Backend.ipynb:101:    " hv.Points(data)(style=point_opts) +\n",
./doc/Tutorials/Bokeh_Backend.ipynb:102:    " hv.Text(6, 0, 'Here is some text')(style=text_opts))"
./doc/Tutorials/Bokeh_Backend.ipynb:164:    "hv.Layout([points.relabel(group=group)(plot=opts)\n",
./doc/Tutorials/Columnar_Data.ipynb:702:    "hv.Overlay([gdp_curves.collapse('Country', fn).relabel(name)(style=dict(linestyle=ls))\n",
./doc/Tutorials/Sampling_Data.ipynb:255:    "labeled_img = img * img_coords * hv.Points([img.closest([(5.1,4.9)])])(style=dict(color='r'))\n",
./doc/Tutorials/Sampling_Data.ipynb:342:    "all_samples = obs_hmap.table().to.scatter3d()(style=dict(alpha=0.15))\n",
./doc/Tutorials/Sampling_Data.ipynb:344:    "subsamples = sampled.to.scatter3d()(style=sample_style)\n",
./doc/Tutorials/Sampling_Data.ipynb:362:    "subsamples = sampled.to.scatter3d()(style=sample_style)\n",
./doc/Tutorials/Sampling_Data.ipynb:390:    "sampling = all_samples * sampled.to.points(extents=extents)(style=dict(color='r'))\n",
./doc/Tutorials/Sampling_Data.ipynb:408:    "sampling = all_samples * sampled.to.points(extents=extents)(style=dict(color='r'))\n",
./doc/Tutorials/Streams.ipynb:552:    "dmap = hv.DynamicMap(sample_generator, streams=[Stream.define('Next')()])\n",
./doc/Tutorials/Bokeh_Elements.ipynb:620:    "hv.NdOverlay({i: hv.Spikes(np.random.randint(0, 100, 10), kdims=['Time'])(plot=dict(position=0.1*i))\n",
./doc/Tutorials/Bokeh_Elements.ipynb:621:    "              for i in range(10)})(plot=dict(yticks=[((i+1)*0.1-0.05, i) for i in range(10)]))"
./doc/Tutorials/Bokeh_Elements.ipynb:1361:    "(hv.Polygons([rectangle(width=2), rectangle(x=6, width=2)])(style={'fill_color': '#a50d0d'})\n",
./doc/Tutorials/Bokeh_Elements.ipynb:1362:    "* hv.Polygons([rectangle(x=2, height=2), rectangle(x=5, height=2)])(style={'fill_color': '#ffcc00'})\n",
./doc/Tutorials/Bokeh_Elements.ipynb:1363:    "* hv.Polygons([rectangle(x=3, height=2, width=2)])(style={'fill_color': 'cyan'}))"
./doc/Tutorials/Linked_Streams.ipynb:181:    "hv.DynamicMap(integral, streams=[streams.Stream.define('Time', time=1.0)(), \n",
./doc/Tutorials/Options.ipynb:396:    "(image + curve)(style={'Image.Function.Sine': dict(cmap='Reds'), 'Curve': dict(color='indianred')})"
./doc/Tutorials/Elements.ipynb:426:    "              kdims=['Group', 'Category'], vdims=['Value'])(style=style).sort()"
./doc/Tutorials/Elements.ipynb:634:    "hv.NdOverlay({i: hv.Spikes(np.random.randint(0, 100, 10), kdims=['Time'])(plot=dict(position=0.1*i))\n",
./doc/Tutorials/Elements.ipynb:635:    "              for i in range(10)})(plot=dict(yticks=[((i+1)*0.1-0.05, i) for i in range(10)]))"
./doc/Tutorials/Elements.ipynb:1397:    "(hv.Polygons([rectangle(width=2), rectangle(x=6, width=2)])(style={'facecolor': '#a50d0d'})\n",
./doc/Tutorials/Elements.ipynb:1398:    "* hv.Polygons([rectangle(x=2, height=2), rectangle(x=5, height=2)])(style={'facecolor': '#ffcc00'})\n",
./doc/Tutorials/Elements.ipynb:1399:    "* hv.Polygons([rectangle(x=3, height=2, width=2)])(style={'facecolor': 'c', 'hatch':'x'}))"

I will go through these notebooks now and update them.

@jlstevens
Copy link
Contributor Author

The grep lines mentioning the stream notebooks are false positives. All the warnings on travis should now be fixed though there still seems to be one in Showcase somewhere that did not show up in grep (applying to a HoloMap?)

@jlstevens
Copy link
Contributor Author

There are no more warnings on Travis and the tests are green - now to fix all the notebooks in examples/!

@jlstevens
Copy link
Contributor Author

jlstevens commented Aug 3, 2017

I'll now work through the uses in reference which should largely mirror the changes to the tutorial notebooks, at least when it comes to the elements:

./examples/reference/elements/bokeh/Polygons.ipynb:66: "(hv.Polygons([rectangle(width=2), rectangle(x=6, width=2)])(style={'fill_color': '#a50d0d'})\n",
./examples/reference/elements/bokeh/Polygons.ipynb:67: " hv.Polygons([rectangle(x=2, height=2), rectangle(x=5, height=2)])(style={'fill_color': '#ffcc00'})\n",
./examples/reference/elements/bokeh/Polygons.ipynb:68: " hv.Polygons([rectangle(x=3, height=2, width=2)])(style={'fill_color': 'cyan'}))"
./examples/reference/elements/bokeh/Spikes.ipynb:96: "hv.NdOverlay({i: hv.Spikes(np.random.randint(0, 100, 10), kdims=['Time'])(plot=dict(position=0.1i))\n",
./examples/reference/elements/bokeh/Spikes.ipynb:97: " for i in range(10)})(plot=dict(yticks=[((i+1)0.1-0.05, i) for i in range(10)]))"
./examples/reference/elements/bokeh/Curve.ipynb:74: "hv.NdOverlay({interp: hv.Curve(points[::8])(plot=dict(interpolation=interp))\n",
./examples/reference/elements/plotly/Curve.ipynb:73: "hv.NdOverlay({interp: hv.Curve(points[::8])(plot=dict(interpolation=interp))\n",
./examples/reference/elements/matplotlib/Polygons.ipynb:66: "(hv.Polygons([rectangle(width=2), rectangle(x=6, width=2)])(style={'facecolor': '#a50d0d'})\n",
./examples/reference/elements/matplotlib/Polygons.ipynb:67: " hv.Polygons([rectangle(x=2, height=2), rectangle(x=5, height=2)])(style={'facecolor': '#ffcc00'})\n",
./examples/reference/elements/matplotlib/Polygons.ipynb:68: " hv.Polygons([rectangle(x=3, height=2, width=2)])(style={'facecolor': 'cyan'}))"
./examples/reference/elements/matplotlib/Spikes.ipynb:96: "hv.NdOverlay({i: hv.Spikes(np.random.randint(0, 100, 10), kdims=['Time'])(plot=dict(position=0.1*i))\n",
./examples/reference/elements/matplotlib/Spikes.ipynb:97: " for i in range(10)})(plot=dict(yticks=[((i+1)*0.1-0.05, i) for i in range(10)]))"
./examples/reference/elements/matplotlib/Curve.ipynb:74: "hv.NdOverlay({interp: hv.Curve(points[::8])(plot=dict(interpolation=interp))\n",
./examples/reference/streams/bokeh/Selection1D_points.ipynb:50: " return selected.relabel(label)(style=dict(color='red'))\n",

@jlstevens
Copy link
Contributor Author

I have checked the grep results above and found that all these uses have been fixed already.

@jlstevens
Copy link
Contributor Author

I'll now make sure these are all fixed (everything left except user guide):

./examples/topics/simulation/sri_model.ipynb:636: " q, [], group=q)(style=dict(cmap=c)).hist()\n",
./examples/topics/simulation/boids.ipynb:225: "dmap = hv.DynamicMap(flock, streams=[Stream.define('Next')()])\n",
./examples/gallery/demos/bokeh/measles_example.ipynb:74: "(heatmap + agg * vline * marker)(opts).cols(1)"
./examples/gallery/demos/bokeh/histogram_example.ipynb:127: "(norm + lognorm + gamma + beta + weibull)(opts).cols(2)"
./examples/gallery/demos/bokeh/stocks_example.ipynb:81: "((aapl * goog * ibm * msft)(plot=plot_opts) +\n",
./examples/gallery/demos/bokeh/stocks_example.ipynb:82: "(avg_scatter(style=scatter_style) * avg_curve(style=curve_style))(plot=plot_opts))"
./examples/gallery/demos/bokeh/area_chart.ipynb:64: "overlay = (python * pypy * jython)(style={'Area': style})\n",
./examples/gallery/demos/bokeh/legend_example.ipynb:62: " hv.Curve(scatter2)(style=dict(line_dash=(4, 4), color='orange')) \n",
./examples/gallery/demos/bokeh/bachelors_degrees_by_gender.ipynb:85: " deg, halign='left', fontsize=10)(style=dict(color=col))\n",
./examples/gallery/demos/matplotlib/measles_example.ipynb:77: "(heatmap + agg * vline * marker)(opts).cols(1)"
./examples/gallery/demos/matplotlib/histogram_example.ipynb:127: "(norm + lognorm + gamma + beta + weibull)(opts).cols(2)"
./examples/gallery/demos/matplotlib/stocks_example.ipynb:82: "(aapl * goog * ibm * msft)(plot=plot_opts) +\\n",
./examples/gallery/demos/matplotlib/stocks_example.ipynb:83: "(avg_scatter(style=scatter_style) * avg_curve(style=curve_style))(plot=plot_opts)"
./examples/gallery/demos/matplotlib/area_chart.ipynb:65: "overlay = (python * pypy * jython)(style={'Area': style})\n",
./examples/gallery/demos/matplotlib/legend_example.ipynb:62: " hv.Curve(scatter2)(style=dict(linestyle='--', color='orange')) \n",
./examples/gallery/demos/matplotlib/bachelors_degrees_by_gender.ipynb:86: " deg, halign='left', fontsize=10)(style=dict(color=col))\n",

@jlstevens
Copy link
Contributor Author

Other some uses of __call__ in the legend_example notebooks, the above lines were all already updated.

@jlstevens
Copy link
Contributor Author

Finally, to check the user guide:

./examples/user_guide/14-Large_Data.ipynb:228:    "color_points = hv.NdOverlay({k: hv.Points([0,0], label=str(k))(style=dict(color=v)) for k, v in color_key})\n",
./examples/user_guide/09-Indexing_and_Selecting_Data.ipynb:366:    "labeled_img = img * img_coords * hv.Points([img.closest([(5.1,4.9)])])(style=dict(color='r'))\n",
./examples/user_guide/09-Indexing_and_Selecting_Data.ipynb:454:    "all_samples = obs_hmap.table().to.scatter3d()(style=dict(alpha=0.15))\n",
./examples/user_guide/09-Indexing_and_Selecting_Data.ipynb:456:    "subsamples = sampled.to.scatter3d()(style=sample_style)\n",
./examples/user_guide/09-Indexing_and_Selecting_Data.ipynb:475:    "subsamples = sampled.to.scatter3d()(style=sample_style)\n",
./examples/user_guide/09-Indexing_and_Selecting_Data.ipynb:503:    "sampling = all_samples * sampled.to.points(extents=extents)(style=dict(color='r'))\n",
./examples/user_guide/09-Indexing_and_Selecting_Data.ipynb:521:    "sampling = all_samples * sampled.to.points(extents=extents)(style=dict(color='r'))\n",
./examples/user_guide/12-Custom_Interactivity.ipynb:242:    "hv.DynamicMap(integral, streams=[streams.Stream.define('Time', time=1.0)(), \n",
./examples/user_guide/Plotting_with_Bokeh.ipynb:107:    "(hv.Curve(data)(style=curve_opts) +\n",
./examples/user_guide/Plotting_with_Bokeh.ipynb:108:    " hv.Points(data)(style=point_opts) +\n",
./examples/user_guide/Plotting_with_Bokeh.ipynb:109:    " hv.Text(6, 0, 'Here is some text')(style=text_opts))"
./examples/user_guide/Plotting_with_Bokeh.ipynb:171:    "hv.Layout([points.relabel(group=group)(plot=opts)\n",
./examples/user_guide/11-Responding_to_Events.ipynb:565:    "dmap = hv.DynamicMap(sample_generator, streams=[Stream.define('Next')()])\n",
./examples/user_guide/Deploying_Bokeh_Apps.ipynb:76:    "    return points.clone(arr, label=label)(style=dict(color='red'))\n",
./examples/user_guide/Deploying_Bokeh_Apps.ipynb:288:    "points = hv.Points(np.random.randn(1000,2 ))(plot=dict(tools=['box_select', 'lasso_select']))\n",
./examples/user_guide/Deploying_Bokeh_Apps.ipynb:297:    "    return points.clone(arr, label=label)(style=dict(color='red'))\n",
./examples/user_guide/Deploying_Bokeh_Apps.ipynb:339:    "    return hv.Curve((xs, np.sin(frequency*xs+phase)*amplitude))(plot=dict(width=800))\n",
./examples/user_guide/Deploying_Bokeh_Apps.ipynb:501:    "    return hv.Curve((xs, np.sin(xs+phase)))(plot=dict(width=800))\n",
./examples/user_guide/Deploying_Bokeh_Apps.ipynb:567:    "    return hv.Curve((xs, np.sin(xs+phase)))(plot=dict(width=800))\n",
./examples/user_guide/Deploying_Bokeh_Apps.ipynb:569:    "stream = hv.streams.Stream.define('Phase', phase=0.)()\n",

@jlstevens
Copy link
Contributor Author

The user guide was updated so I just updated a few unit tests, concluding all the replacements I know of.

@jlstevens
Copy link
Contributor Author

@philippjfr Once the tests go green, I think this is ready to merge. Do you know of anywhere else in the codebase that __call__ might still be hanging around?

@jbednar
Copy link
Member

jbednar commented Aug 3, 2017

I'm sure it needs fixing in some .ipynb files on datashader, and maybe on paramnb.

@jlstevens
Copy link
Contributor Author

jlstevens commented Aug 3, 2017

@jbednar I was only thinking of the HoloViews codebase for now.

Do you think I should enable the warnings by default for everyone else? i.e for other libraries and user code?

@jlstevens
Copy link
Contributor Author

Actually, thinking about it, we shouldn't introduce those warnings in a minor release. I think this is ready to merge if __call__ is eliminated from the holoviews codebase.

@jbednar
Copy link
Member

jbednar commented Aug 3, 2017

At some point, yes...

@jlstevens
Copy link
Contributor Author

At some point, yes...

Namely the 1.9 release.

@philippjfr
Copy link
Member

Looks fine, ready to merge?

@jlstevens
Copy link
Contributor Author

Yes.

@philippjfr philippjfr merged commit d6308ca into master Aug 3, 2017
ea42gh pushed a commit to ea42gh/holoviews that referenced this pull request Aug 12, 2017
* Enabled warnings about the use of __call__ on travis

* Replaced uses of __call__ with opts in Bokeh Elements notebook

* Replaced uses of __call__ with opts in Columnar Data notebook

* Replaced uses of __call__ with opts in Elements notebook

* Replaced uses of __call__ with opts in Options notebook

* Replaced uses of __call__ with opts in Sampling Data notebook

* Replaced remaining uses of __call__ in doc/Tutorials

* Replaced uses of __call__ in the legend_example notebooks

* Replaced uses of __call__ with .opts in store option tests
@jlstevens jlstevens added this to the 1.8.2 milestone Aug 21, 2017
@philippjfr philippjfr deleted the travis_call_warnings branch August 21, 2017 20:57
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants