Enable Dimension to accept a dict spec #5333

stanwest · 2022-06-08T21:04:28Z

This PR closes #5332 and is closely related to issue #2315 and PR #2332. Its main changes are as follows:

Enable one to specify a Dimension with a dict.
When spec is a dict that lacks a "name" key, raise ValueError.
When spec has an invalid type, raise ValueError. (Although TypeError would be customary, ValueError is backward-compatible with the behavior of helper functions around Dimension.)
Look up preset dimensions only when spec is a str.
Let the param system handle wrong types of parameters.

One could further refine this code as follows:

It's unclear whether the Dimension.presets mapping (of PR Initial prototype for Dimension preset system #644) should remain; it appears unused internally (including by tests) and undocumented.
I'd rather see checks for instances of collections.abc.Mapping instead of dict, but other related code in Holoviews uses dict.
When a caller calls the Dimension class, dimension_name function, or process_dimensions function with an invalid type, raise TypeError instead of the current ValueError.

This change might make it easier for hvplot to accept kdims specified as dicts instead of Dimensions.

jlstevens · 2022-07-04T16:14:48Z

Sorry for not getting back to you sooner! I've approved your workflow so we can run the tests.

As for the proposal itself, I think it does make sense and that what you've written is accurate. However, the Dimension constructor has a lot of history (as you've noticed!) so we need to be careful to identify which parts can be safely ignored and deprecated.

At any rate, I do think this is an important suggestion to consider. As HoloViews 1.15.0 is in a feature freeze (currently an rc), I will add it to the 1.15.1 milestone so we can think about including it in that release.

stanwest · 2022-07-13T15:23:36Z

Might an unrelated issue be interfering with the run of the CI tests? The failed tests appear unrelated to the changes in this pull request.

The following 12 tests failed on all Python versions and all operating systems:

holoviews/tests/operation/test_datashader.py::DatashaderShadeTests::test_shade_categorical_images_grid
holoviews/tests/operation/test_datashader.py::DatashaderShadeTests::test_shade_categorical_images_xarray
holoviews/tests/operation/test_datashader.py::DatashaderRasterizeTests::test_rasterize_dask_trimesh
holoviews/tests/operation/test_datashader.py::DatashaderRasterizeTests::test_rasterize_dask_trimesh_implicit_nodes
holoviews/tests/operation/test_datashader.py::DatashaderRasterizeTests::test_rasterize_dask_trimesh_with_node_vdims
holoviews/tests/operation/test_datashader.py::DatashaderRasterizeTests::test_rasterize_pandas_trimesh_implicit_nodes
holoviews/tests/operation/test_datashader.py::DatashaderRasterizeTests::test_rasterize_trimesh
holoviews/tests/operation/test_datashader.py::DatashaderRasterizeTests::test_rasterize_trimesh_ds_aggregator
holoviews/tests/operation/test_datashader.py::DatashaderRasterizeTests::test_rasterize_trimesh_node_explicit_vdim
holoviews/tests/operation/test_datashader.py::DatashaderRasterizeTests::test_rasterize_trimesh_node_vdim_precedence
holoviews/tests/operation/test_datashader.py::DatashaderRasterizeTests::test_rasterize_trimesh_string_aggregator
holoviews/tests/operation/test_datashader.py::DatashaderRasterizeTests::test_rasterize_trimesh_vertex_vdims

The following 5 tests also failed on Python 3.8 and 3.9 on Ubuntu:

holoviews/tests/plotting/bokeh/test_errorbarplot.py::TestErrorBarsPlot::test_errorbars_padding_logy
holoviews/tests/plotting/bokeh/test_histogramplot.py::TestSideHistogramPlot::test_histogram_padding_logx
holoviews/tests/plotting/bokeh/test_spreadplot.py::TestSpreadPlot::test_spread_padding_logy
holoviews/tests/plotting/matplotlib/test_histogramplot.py::TestHistogramPlot::test_histogram_padding_logx
holoviews/tests/plotting/matplotlib/test_scatter3d.py::TestPointPlot::test_scatter3d_padding_logz

stanwest · 2022-07-13T21:40:06Z

I may have figured it out: #5344 updated the tests for datashader 0.14.1. Shall I rebase to incorporate that PR?

jlstevens · 2022-07-13T21:54:41Z

That sounds very plausible and yes, please rebase. Given we just had a release, it would be good to catch up and we know all the tests have been passing on mater.

Also test constructing MultidimensionalMapping with kdim given as dict.

Enables specifying Dimension with dict containing name and label.

Also, use name as default label.

Let the param system handle (1) a non-string name or label within a tuple and (2) a non-hashable label, which previously raised "TypeError: unhashable type" at the membership check.

Two helper functions no longer need to raise such an exception.

philippjfr · 2022-09-26T11:47:30Z

Not sure why the tests didn't re-run on this one.

jlstevens · 2022-09-26T14:01:20Z

Right, I can't see the test status...

stanwest · 2022-09-26T14:33:23Z

I also see no tests, but let me know if anything is needed from me.

philippjfr · 2022-09-26T14:35:13Z

Sorry for leaving this for so long @stanwest. If you could try pushing an empty commit or something to try to trigger the test suite that would be helpful.

codecov-commenter · 2022-09-27T18:06:32Z

Codecov Report

Merging #5333 (af4b20a) into master (2d2b728) will increase coverage by 0.05%.
The diff coverage is 84.44%.

@@            Coverage Diff             @@
##           master    #5333      +/-   ##
==========================================
+ Coverage   88.07%   88.12%   +0.05%     
==========================================
  Files         301      301              
  Lines       61957    62108     +151     
==========================================
+ Hits        54567    54735     +168     
+ Misses       7390     7373      -17

Impacted Files	Coverage Δ
holoviews/core/dimension.py	`86.47% <75.86%> (+0.13%)`	⬆️
holoviews/tests/core/test_dimensions.py	`99.55% <100.00%> (+0.03%)`	⬆️
holoviews/tests/core/test_ndmapping.py	`99.47% <100.00%> (+<0.01%)`	⬆️
holoviews/core/options.py	`82.44% <0.00%> (-0.36%)`	⬇️
holoviews/core/data/pandas.py	`93.30% <0.00%> (-0.10%)`	⬇️
holoviews/pyodide.py	`0.00% <0.00%> (ø)`
holoviews/__init__.py	`72.22% <0.00%> (ø)`
holoviews/plotting/links.py	`94.73% <0.00%> (ø)`
holoviews/tests/conftest.py	`100.00% <0.00%> (ø)`
... and 16 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

philippjfr · 2022-09-27T18:08:25Z

Looks good to me and tests seem to be passing but I'll have @jlstevens have the final say.

jlstevens · 2022-09-27T20:50:00Z

Given that the imminent release is a minor version bump and should only be bug fixes, I propose that we merge this PR immediately afterwards. Overall it looks good and I look forward to trying it out on master!

stanwest · 2022-09-28T10:43:31Z

Thanks!

maximlt · 2022-10-15T08:44:25Z

Given that this PR was approved and HoloViews 1.15.1 was released, I'm going to merge it.

github-actions · 2024-10-23T20:33:23Z

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.

jlstevens added this to the 1.15.1 milestone Jul 4, 2022

stanwest added 8 commits July 13, 2022 22:36

Add failing tests of specifying Dimension with dict

102ae2e

Also test constructing MultidimensionalMapping with kdim given as dict.

Look up preset Dimension only with str spec

85f9f42

Enables specifying Dimension with dict containing name and label.

Require "name" key in Dimension dict

46af270

Also, use name as default label.

Consolidate handling of types of Dimension specs

df5ee69

Gather params into one dict instead of two

b5cd405

Allow wrong types to fall through to param checks

9a21842

Let the param system handle (1) a non-string name or label within a tuple and (2) a non-hashable label, which previously raised "TypeError: unhashable type" at the membership check.

Improve handling of name and label in tuple

c47b043

Raise when calling Dimension with invalid type

fcb3180

Two helper functions no longer need to raise such an exception.

stanwest force-pushed the init_dimension_dict branch from a849a7c to fcb3180 Compare July 14, 2022 02:55

Reformat parenthesis

af4b20a

maximlt modified the milestones: 1.15.1, 1.15.2 Oct 14, 2022

maximlt merged commit 8b66538 into holoviz:master Oct 15, 2022

jlstevens modified the milestones: 1.15.x, 1.15.2 Oct 24, 2022

stanwest deleted the init_dimension_dict branch December 1, 2022 19:00

github-actions bot locked as resolved and limited conversation to collaborators Oct 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable Dimension to accept a dict spec #5333

Enable Dimension to accept a dict spec #5333

stanwest commented Jun 8, 2022 •

edited

Loading

jlstevens commented Jul 4, 2022

stanwest commented Jul 13, 2022

stanwest commented Jul 13, 2022

jlstevens commented Jul 13, 2022

philippjfr commented Sep 26, 2022

jlstevens commented Sep 26, 2022

stanwest commented Sep 26, 2022

philippjfr commented Sep 26, 2022

codecov-commenter commented Sep 27, 2022 •

edited

Loading

philippjfr commented Sep 27, 2022

jlstevens commented Sep 27, 2022

stanwest commented Sep 28, 2022

maximlt commented Oct 15, 2022

github-actions bot commented Oct 23, 2024

Enable Dimension to accept a dict spec #5333

Enable Dimension to accept a dict spec #5333

Conversation

stanwest commented Jun 8, 2022 • edited Loading

jlstevens commented Jul 4, 2022

stanwest commented Jul 13, 2022

stanwest commented Jul 13, 2022

jlstevens commented Jul 13, 2022

philippjfr commented Sep 26, 2022

jlstevens commented Sep 26, 2022

stanwest commented Sep 26, 2022

philippjfr commented Sep 26, 2022

codecov-commenter commented Sep 27, 2022 • edited Loading

Codecov Report

philippjfr commented Sep 27, 2022

jlstevens commented Sep 27, 2022

stanwest commented Sep 28, 2022

maximlt commented Oct 15, 2022

github-actions bot commented Oct 23, 2024

stanwest commented Jun 8, 2022 •

edited

Loading

codecov-commenter commented Sep 27, 2022 •

edited

Loading